Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: barebones dependency conversion #61

Merged
merged 5 commits into from
Dec 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17,882 changes: 17,575 additions & 307 deletions dist/index.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

51 changes: 51 additions & 0 deletions dist/licenses.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,14 @@
"@commitlint/cli": "17.8.0",
"@commitlint/config-conventional": "17.8.0",
"fs-extra": "11.1.1",
"husky": "8.0.3"
"husky": "8.0.3",
"lodash": "4.17.21"
},
"devDependencies": {
"@types/chai": "4.3.5",
"@types/fs-extra": "11.0.1",
"@types/glob": "8.1.0",
"@types/lodash": "4.17.13",
"@types/mocha": "10.0.1",
"@types/mock-fs": "4.13.1",
"@types/node": "20.8.6",
Expand Down
3 changes: 3 additions & 0 deletions src/createPackage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { execSync } from 'child_process';
import { copySolidityFiles } from './copySolidityFiles';
import { PackageJson } from './types';
import { createReadmeAndLicense } from './createReadmeAndLicense';
import { parseGitmodulesDependencies } from './parseGitmodulesDependencies';
import { ExportType } from './constants';

export const createPackage = (
Expand All @@ -25,12 +26,14 @@ export const createPackage = (
// Read and copy the input package.json
const inputPackageJson = fse.readJsonSync('./package.json');
if (!inputPackageJson) throw new Error('package.json not found');
const gitDependencies = parseGitmodulesDependencies();
// Create custom package.json in the export directory
const packageJson: PackageJson = {
name: packageFullName,
version: inputPackageJson.version,
dependencies: {
...inputPackageJson.dependencies,
...gitDependencies,
},
};
fse.writeJsonSync(`${destinationDir}/package.json`, packageJson, { spaces: 4 });
Expand Down
23 changes: 23 additions & 0 deletions src/parseGitmodulesDependencies.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { chunk } from 'lodash';
import fse from 'fs-extra';

export const parseGitmodulesDependencies = (): { [key: string]: string } => {
let iniAsStr;
try {
iniAsStr = fse.readFileSync('./.gitmodules', { encoding: 'utf8' });
} catch {
return {};
}
const matches = [...iniAsStr.matchAll(/(?:path|url) = (.*)/g)].map(regexMatch => regexMatch[1]);
if (matches.length == 0) return {};

// this assumes .gitmodules to be well formed, same amount of paths and urls
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If is not, shouldn't it throw an error describing the issue?
I see this as a NTH tho

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

52b257b
added a test for a valid .gitmodules that breaks it, but am not explicitly throwing an error in that case, as figuring out if the precondition is met or not would involve extra logic

const dependencyPairs = chunk(matches, 2);

// and path is always before url
const dependencyObject = dependencyPairs.reduce<{ [key: string]: string }>((acc, curr) => {
acc[curr[0].replace('lib/', '')] = curr[1];
return acc;
}, {});
return dependencyObject;
};
2 changes: 1 addition & 1 deletion src/transformRemappings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const transformRemappings = (file: string): string => {
line = line.replace(remappingOrigin, remappingDestination);
}

line = line.replace(/(['""]).*node_modules\//, `$1`);
line = line.replace(/(['""]).*(?:node_modules|lib)\//, `$1`);

return line;
})
Expand Down
129 changes: 129 additions & 0 deletions test/unit/parseGitmodulesDependencies.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import { expect } from 'chai';
import mock from 'mock-fs';
import { parseGitmodulesDependencies } from '../../src/parseGitmodulesDependencies';

describe('parseGitmodulesDependencies', () => {
before(() => {
mock.restore();
});
after(() => {
mock.restore();
});

describe('with no gitmodules file', function () {
it('should return an empty dependency list', function () {
const parsedDependencies = parseGitmodulesDependencies();
expect(parsedDependencies).to.deep.eq({});
});
});

describe('with an empty file', function () {
before(() => {
mock({
'.gitmodules': '',
});
});
it('should return an empty dependency list', function () {
const parsedDependencies = parseGitmodulesDependencies();
expect(parsedDependencies).to.deep.eq({});
});
});

describe('with a file with several dependencies', function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe worth also having a case where the gitmodules exist but is not in the format we expect it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added them in 52b257b

before(() => {
mock({
'.gitmodules': `
[submodule "lib/forge-std"]
path = lib/forge-std
url = https://github.com/foundry-rs/forge-std
[submodule "lib/openzeppelin-contracts"]
path = lib/openzeppelin-contracts
url = https://github.com/OpenZeppelin/openzeppelin-contracts
`,
});
});
it('should be able to parse them', function () {
const parsedDependencies = parseGitmodulesDependencies();
expect(parsedDependencies['forge-std']).to.eq('https://github.com/foundry-rs/forge-std');
expect(parsedDependencies['openzeppelin-contracts']).to.eq(
'https://github.com/OpenZeppelin/openzeppelin-contracts',
);
expect(Object.keys(parsedDependencies)).to.have.length(2);
});
});

describe('with a file with extra (valid) keys besides path and url', function () {
before(() => {
mock({
'.gitmodules': `
[submodule "lib/forge-std"]
path = lib/forge-std
branch = .
url = https://github.com/foundry-rs/forge-std
[submodule "lib/openzeppelin-contracts"]
ignore = all
path = lib/openzeppelin-contracts
url = https://github.com/OpenZeppelin/openzeppelin-contracts
`,
});
});
it('should be able to parse them', function () {
const parsedDependencies = parseGitmodulesDependencies();
expect(parsedDependencies['forge-std']).to.eq('https://github.com/foundry-rs/forge-std');
expect(parsedDependencies['openzeppelin-contracts']).to.eq(
'https://github.com/OpenZeppelin/openzeppelin-contracts',
);
expect(Object.keys(parsedDependencies)).to.have.length(2);
});
});

describe('with a file where subsection keys are specified in multiple subsection definitions', function () {
before(() => {
// this is valid syntax according to git, but our regex 'parser' can't handle it
mock({
'.gitmodules': `
[submodule "lib/openzeppelin-contracts"]
url = https://github.com/OpenZeppelin/openzeppelin-contracts
[submodule "lib/forge-std"]
path = lib/forge-std
[submodule "lib/openzeppelin-contracts"]
path = lib/openzeppelin-contracts
[submodule "lib/forge-std"]
url = https://github.com/foundry-rs/forge-std
`,
});
});
it.skip('should be able to parse them', function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are some tests being skipped?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to acknowledge, yet thoroughly document the limitations of the current implementation. Example of a previous discussion: #58 (comment)

This test in particular was added to document the consequences of not doing proper git-config format parsing (I couldn't find an existing implementation, and the generic .ini parser in npm was not enough either)

const parsedDependencies = parseGitmodulesDependencies();
expect(parsedDependencies['forge-std']).to.eq('https://github.com/foundry-rs/forge-std');
expect(parsedDependencies['openzeppelin-contracts']).to.eq(
'https://github.com/OpenZeppelin/openzeppelin-contracts',
);
expect(Object.keys(parsedDependencies)).to.have.length(2);
});
});

describe('with a file urls come before paths', function () {
before(() => {
// this is valid syntax according to git, but our regex 'parser' can't handle it
mock({
'.gitmodules': `
[submodule "lib/openzeppelin-contracts"]
url = https://github.com/OpenZeppelin/openzeppelin-contracts
path = lib/openzeppelin-contracts
[submodule "lib/forge-std"]
url = https://github.com/foundry-rs/forge-std
path = lib/forge-std
`,
});
});
it.skip('should be able to parse them', function () {
const parsedDependencies = parseGitmodulesDependencies();
expect(parsedDependencies['forge-std']).to.eq('https://github.com/foundry-rs/forge-std');
expect(parsedDependencies['openzeppelin-contracts']).to.eq(
'https://github.com/OpenZeppelin/openzeppelin-contracts',
);
expect(Object.keys(parsedDependencies)).to.have.length(2);
});
});
});
31 changes: 31 additions & 0 deletions test/unit/transformRemappings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,37 @@ describe('transformRemappings', () => {
});
});

describe('with remapping for path inside lib/', function () {
before(() => {
mock({
'remappings.txt': `
solmate/=lib/solmate/src/`,
});
});

it('should apply remapping and then remove lib from path', () => {
const fileContent = `
import {FixedPointMathLib} from 'solmate/utils/FixedPointMathLib.sol';
import 'solmate/utils/FixedPointMathLib.sol';
import "solmate/utils/FixedPointMathLib.sol";
import {FixedPointMathLib as FPL} from 'solmate/utils/FixedPointMathLib.sol';
import * as FPL from "solmate/utils/FixedPointMathLib.sol";
`;

const transformedContent = transformRemappings(fileContent);

expect(transformedContent).to.include(
`import {FixedPointMathLib} from 'solmate/src/utils/FixedPointMathLib.sol';`,
);
expect(transformedContent).to.include(`import 'solmate/src/utils/FixedPointMathLib.sol';`);
expect(transformedContent).to.include(`import "solmate/src/utils/FixedPointMathLib.sol";`);
expect(transformedContent).to.include(
`import {FixedPointMathLib as FPL} from 'solmate/src/utils/FixedPointMathLib.sol';`,
);
expect(transformedContent).to.include(`import * as FPL from "solmate/src/utils/FixedPointMathLib.sol";`);
});
});

it('should leave unrelated imports alone', () => {
const fileContent = `
import './Contract.sol';
Expand Down
Loading
Loading