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

Fix release truncation issue by changing symlinks #32

Merged
merged 1 commit into from
Jul 7, 2020

Conversation

cameel
Copy link
Member

@cameel cameel commented Jul 6, 2020

Resolves ethereum/solidity#9261. Part of ethereum/solidity#9258.

It's a PR to master but we should backport the fix to gh-pages too.

Cause of the problem

See diuscussion on Gitter

@cameel

ls -la wasm/soljson-*.js
lrwxrwxrwx 1 cameel cameel 34 2020-07-06 17:33 wasm/soljson-latest.js -> soljson-v0.6.10+commit.00c0fcaf.js
lrwxrwxrwx 1 cameel cameel 34 2020-07-06 17:33 wasm/soljson-nightly.js -> soljson-v0.6.10+commit.00c0fcaf.js
lrwxrwxrwx 1 cameel cameel 41 2020-06-30 14:44 wasm/soljson-v0.6.10+commit.00c0fcaf.js -> ../bin/soljson-v0.6.10+commit.00c0fcaf.js

Looks like the solc-bin commit that added Release 0.6.10 replaced wasm/soljson-latest.js and wasm/soljson-nightly.js with symlinks back to files in bin/.

The update script has not been updated to account for that so it still just overwrites them instead of updating the links. They both point at the same file and there are also asynchronous operations trying to read with from the file they're writing to.

So making things synchronous should fix this. But we should probably make the script take links into account instead.

Also, not sure if it's deliberate, but wasm/soljson-nightly.js points at the release file, not at a nightly build. Technically, it's the latest build available in the wasm/ directory but I suspect this was not the intention. Should we really have soljson-nightly.js in that dir if it nightlies are not linked there?

@ekpyron

We should (1) remove wasm/soljson-nightly.js, (2) just keep wasm/soljson-latest.js be a symlink to bin/soljson-latest.js that never changes and (3) remove all logic for soljson-nightly and soljson-latest for wasm/ from the update script.

… soljson.js with static links to bin/

- This fixes the issue of the `update` script damaging the release files by doing parallel reads and writes through the links in wasm/ instead of just updating the links.
@cameel cameel requested review from ekpyron and chriseth July 6, 2020 18:31
@cameel cameel self-assigned this Jul 6, 2020
Copy link
Member

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

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

Looks good, if the tests pass!

Copy link
Contributor

@chriseth chriseth left a comment

Choose a reason for hiding this comment

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

Wouldn't it make sense to make all the '-latestand-nightly` files symlinks?

@chriseth
Copy link
Contributor

chriseth commented Jul 7, 2020

Ah, that seems to be #33!

@chriseth chriseth merged commit 16d460b into master Jul 7, 2020
@cameel
Copy link
Member Author

cameel commented Jul 7, 2020

Yeah. It wasn't essential to the fix and required more than just removing a few lines from the update script so I put it in a separate PR.

@cameel cameel deleted the fix-update-script-damaging-releases-via-symlinks branch July 7, 2020 09:10
@cameel
Copy link
Member Author

cameel commented Jul 7, 2020

I just merged the master branch (with this PR in it) back into gh-pages. Removing master from the repo now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants