Skip to content

Support broader range of dependency versions #758

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

Draft
wants to merge 10 commits into
base: testing-min-dependencies
Choose a base branch
from
Draft
32 changes: 27 additions & 5 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,19 @@ workflows:
pr-checks:
jobs:
- check-coding-style
- node-v10
- node-v12
- node-v12:
name: node-v12-min-dependencies
min_dependencies: true
- node-v14
- node-v16
- node-v18
- node-v20
- node-current:
run_coveralls: true
- node-current:
name: node-current-min-dependencies
min_dependencies: true
- build-package
- hardhat-sample-project: *requires_package
- cli-smoke-test: *requires_package
Expand Down Expand Up @@ -168,10 +173,31 @@ jobs:
run_coveralls:
type: boolean
default: false
min_dependencies:
description: "Install the oldest dependencies still matching ranges specified in package.json"
type: boolean
default: false
steps:
# We want the default npm here. Older one might not work with older node.js
- show-npm-version
- checkout
- when:
condition: <<parameters.min_dependencies>>
steps:
- run:
name: Install the semver utility
command: |
# NOTE: Newer cimg/node images require sudo here, older don't. Try both.
sudo npm install semver --global || npm install semver --global
- run:
name: Force oldest supported dependency versions in package.json
command: |
min_package_json=$(.circleci/package-json-with-min-dependencies.sh)
echo "$min_package_json" > package.json
- run:
name: "Show selected dependency versions"
command: |
jq 'with_entries(select(.key == "dependencies" or .key == "devDependencies"))' package.json --indent 4
- install-dependencies:
cache-id: solc-js
- run:
Expand Down Expand Up @@ -345,10 +371,6 @@ jobs:
- run: cd solidity/ && curl "https://binaries.soliditylang.org/bin/soljson-nightly.js" --location --output soljson.js
- run: cd solidity/ && test/externalTests/solc-js/solc-js.sh "$(realpath soljson.js)" "$(scripts/get_version.sh)" "$(realpath ../solc-js/)"

node-v10:
<<: *node-base
docker:
- image: cimg/node:10.24
node-v12:
<<: *node-base
docker:
Expand Down
56 changes: 56 additions & 0 deletions .circleci/package-json-with-min-dependencies.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#!/usr/bin/env bash
# Creates a variant of package.json, hard-coded to use the oldest version of each dependency in
# the supported range.
#
# package.json is taken from the current working directory. The script does not modify the file -
# the new version is instead printed to the standard output.
#
# Dependencies: npm, jq, semver

set -euo pipefail

function fail { >&2 echo "$@"; exit 1; }

function find_min_supported_versions {
local packages_json min_versions packages supported_range available_versions min_version

# The function expects a JSON dict with package names and versions, extracted from package.json by the caller.
packages_json=$(cat -)

# Use @tsv filter to get tab-separated package names. We assume that neither packages, nor
# available versions contain spaces. Spaces in version range are fine though.
packages=$(echo "$packages_json" | jq --raw-output 'keys | @tsv')
min_versions=()
for package in $packages; do
available_versions=$(npm view "$package" versions --json | jq --raw-output @tsv)
supported_range=$(echo "$packages_json" | jq --raw-output ".\"${package}\"")

# shellcheck disable=SC2086
# semver prints versions matching the range, one per line, in semver order, oldest first
min_version=$(semver $available_versions --range "$supported_range" | head --lines 1)
[[ $min_version != "" ]] || fail "No version matching ${supported_range} found for package ${package}."

# Debug info. It goes to stderr not to interfere with actual output.
>&2 echo "Package ${package}:"
>&2 echo " minimum version: ${min_version}"
>&2 echo " supported range: ${supported_range}"
>&2 echo " available versions: ${available_versions}"
>&2 echo

min_versions+=("{\"${package}\": \"${min_version}\"}")
done

# Actual output: min_versions merged into a single dict.
echo "${min_versions[@]}" | jq --slurp add
}

dependencies=$(jq .dependencies package.json | find_min_supported_versions)
dev_dependencies=$(jq .devDependencies package.json | find_min_supported_versions)

# Print package.json with overwritten dependency versions
cat <<EOF | jq --slurp '.[0] * .[1]' package.json -
{
"dependencies": ${dependencies},
"devDependencies": ${dev_dependencies}
}
EOF
2 changes: 1 addition & 1 deletion formatters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export function formatFatalError (message) {
type: 'JSONError',
component: 'solcjs',
severity: 'error',
message: message,
message,
formattedMessage: 'Error: ' + message
}
]
Expand Down
48 changes: 24 additions & 24 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,32 +46,32 @@
},
"homepage": "https://github.com/ethereum/solc-js#readme",
"dependencies": {
"command-exists": "^1.2.8",
"commander": "^8.1.0",
"follow-redirects": "^1.12.1",
"js-sha3": "0.8.0",
"memorystream": "^0.3.1",
"semver": "^5.5.0",
"tmp": "0.0.33"
"command-exists": ">=1.1.0",
Copy link
Member Author

@cameel cameel Jan 25, 2025

Choose a reason for hiding this comment

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

Note that I replaced ^ with >= in all version ranges. I'm still on the fence whether that's a good idea or whether I should always put a < there to keep major version updates manual:

Pros of >=:

  • We almost never update these versions and this forces projects using solc-js to use old versions too (or do hacky workarounds). >= forces the update on us by default and we can always restrict with < as needed.
  • There's not much risk of this seriously breaking a downstream app that's already using the package. If it happens, the app can always restrict the dependency in its own package.json. And that's only a concern for the devs of the app, not its users. It may only happen when devs update their dependencies - outside of that the installed versions are locked via package-lock.json.

Cons of >=:

  • Already released versions in npm will stay unlimited forever and eventually stop working with latest versions of dependencies because one of them will inevitably introduce an actual breaking change. This may confuse someone trying to add a dependency on an older version of solc-js to their app.
  • There's some risk of a new breaking version appearing during our release process. Though in solc-js this would be easy to fix quickly, because CI is quick and it gets tagged late in the process.

"commander": ">=8.0.0 <12.0.0",
Copy link
Member Author

@cameel cameel Jan 25, 2025

Choose a reason for hiding this comment

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

Versions >=13.0.0 do not pass our tests. We could probably support them if we wanted but that will require some code changes on our side. Not sure how extensive.

Versions >=12.0.0 trigger failures on older node.js versions due to the use of ?? operator or its variants.

"follow-redirects": ">=1.0.0",
"js-sha3": ">=0.8.0",
"memorystream": ">=0.3.0",
"semver": ">=5.0.0",
"tmp": ">=0.0.26"
},
"devDependencies": {
"@types/node": "^16.11.7",
"@types/semver": "^7.3.9",
"@types/tape": "^4.13.2",
"@types/tmp": "^0.2.3",
"@typescript-eslint/eslint-plugin": "^5.8.0",
"@typescript-eslint/parser": "^5.8.0",
"coveralls": "^3.0.0",
"eslint": "^7.32.0",
"eslint-config-standard": "^16.0.3",
"eslint-plugin-import": "^2.25.3",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-promise": "^5.1.1",
"nyc": "^15.1.0",
"tape": "^4.11.0",
"tape-spawn": "^1.4.2",
"ts-node": "^10.4.0",
"typescript": "^4.5.4"
"@types/node": ">=16.11.7",
"@types/semver": ">=7.1.0",
"@types/tape": ">=4.2.27",
"@types/tmp": ">=0.2.0",
"@typescript-eslint/eslint-plugin": ">=5.0.0 <6.0.0",
"@typescript-eslint/parser": ">=5.0.0 <6.0.0",
"coveralls": ">=3.0.0",
"eslint": ">=7.12.1 <8.0.0",
"eslint-config-standard": ">=16.0.3 <17.0.0",
"eslint-plugin-import": ">=2.25.0",
"eslint-plugin-node": ">=11.1.0",
"eslint-plugin-promise": ">=5.0.0 <7.0.0",
"nyc": ">=15.0.0",
"tape": ">=4.11.0",
"tape-spawn": ">=1.0.0",
"ts-node": ">=10.0.0",
"typescript": ">=4.2.2 <5.0.0"
Comment on lines +62 to +74
Copy link
Member Author

Choose a reason for hiding this comment

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

Typescript, eslint and their plugins here are restricted only because they have issues on node.js 12 and 14. There are failures due to the use of ?? operator or its variants. On the other hand there are no issues on the latest node.js.

Not sure how to handle this. I guess node.js 12 and 14 are past EOL now so it may be ok to just remove the restriction. The issue is that our tests fail, so we would have to patch package.json in our tests.

Another possibility might be to just drop official v12 and v14 support.

},
"nyc": {
"exclude": [
Expand Down
2 changes: 1 addition & 1 deletion smtsolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function solve (query, solver) {
encoding: 'utf8',
maxBuffer: 1024 * 1024 * 1024,
stdio: 'pipe',
timeout: timeout // Enforce timeout on the process, since solvers can sometimes go around it.
timeout // Enforce timeout on the process, since solvers can sometimes go around it.
}
).toString();
} catch (e) {
Expand Down
3 changes: 1 addition & 2 deletions solc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const commanderParseInt = function (value) {
program.name('solcjs');
program.version(solc.version());
program
.option('--version', 'Show version and exit.')
.option('--optimize', 'Enable bytecode optimizer.', false)
.option(
'--optimize-runs <optimize-runs>',
Expand Down Expand Up @@ -201,7 +200,7 @@ const cliInput = {
}
}
},
sources: sources
sources
};
if (program.verbose) { console.log('>>> Compiling:\n' + toFormattedJson(cliInput) + '\n'); }
const output = JSON.parse(solc.compile(JSON.stringify(cliInput), callbacks));
Expand Down
8 changes: 4 additions & 4 deletions test/smtcallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ tape('SMTCheckerCallback', function (t) {
const inputJSON = JSON.stringify({
language: 'Solidity',
sources: input,
settings: settings
settings
});

let tests;
Expand Down Expand Up @@ -192,7 +192,7 @@ tape('SMTCheckerCallback', function (t) {
expectations: expected,
solidity: { test: { content: preamble + source } },
ignoreCex: source.includes('// SMTIgnoreCex: yes'),
engine: engine
engine
};
}

Expand All @@ -214,7 +214,7 @@ tape('SMTCheckerCallback', function (t) {
const engine = test.engine !== undefined ? test.engine : 'all';
settings = {
modelChecker: {
engine: engine,
engine,
solvers: [
'smtlib2'
]
Expand All @@ -225,7 +225,7 @@ tape('SMTCheckerCallback', function (t) {
JSON.stringify({
language: 'Solidity',
sources: test.solidity,
settings: settings
settings
}),
// This test needs z3 specifically.
{ smtSolver: smtchecker.smtCallback(smtsolver.smtSolver, z3HornSolvers[0]) }
Expand Down
2 changes: 1 addition & 1 deletion test/smtchecker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ tape('SMTCheckerWithSolver', function (t) {
const input = {
language: 'Solidity',
sources: source,
settings: settings
settings
};

const output = JSON.parse(solc.compile(JSON.stringify(input)));
Expand Down
2 changes: 1 addition & 1 deletion translate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function translateErrors (ret, errors) {
type = 'Error';
}
ret.push({
type: type,
type,
component: 'general',
severity: (type === 'Warning') ? 'warning' : 'error',
message: errors[error],
Expand Down
4 changes: 2 additions & 2 deletions wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ function compileStandardWrapper (compile, inputRaw, readCallback) {

// Try to wrap around old versions
if (!isNil(compile.compileJsonCallback)) {
const inputJson = JSON.stringify({ sources: sources });
const inputJson = JSON.stringify({ sources });
const output = compile.compileJsonCallback(inputJson, optimize, readCallback);
return translateOutput(output, libraries);
}

if (!isNil(compile.compileJsonMulti)) {
const output = compile.compileJsonMulti(JSON.stringify({ sources: sources }), optimize);
const output = compile.compileJsonMulti(JSON.stringify({ sources }), optimize);
return translateOutput(output, libraries);
}

Expand Down