Ipfs-companion: Reproducible Build

[lidel]

Post-submission review process at Mozilla kicked in for ipfs-companion and requires us to provide source code + build steps that will produce exactly the same code as one published to NPM:

The following information is needed to complete your review:

1) This version contains obfuscated, minified, concatenated or otherwise machine-generated code. Please provide the original sources, together with instructions on how to generate the final XPI. Source code must be provided as an archive and uploaded using the source code upload field, which can be done during submission or on the version page in the developer hub.

Please read through the instructions at https://developer.mozilla.org/en-US/Add-ons/Source_Code_Submission. If sources have not been provided within 7 days, this version will be rejected and removed from our queues.
More Information: https://blog.mozilla.org/addons/2016/04/05/improved-review-time-with-links-to-sources/

Established libraries (eg: JQuery, bootstrap) do not require the inclusion of their sources.
Please use non-custom versions of jquery-ui, as well as the stock version of jquery and others, that match the checksums in
https://github.com/mozilla/amo-validator/blob/master/validator/testcases/hashes-allowed.txt

Problem 1
I am unable to replicate build results for two of four dependencies: ipfs-api and is-ipfs.

I downloaded https://github.com/ipfs/js-ipfs-api/releases/tag/v15.0.1 but was unable to produce the same file as https://unpkg.com/[email protected]/dist/index.min.js -- after running npm install && npm run build minified code looks different. Same goes for is-ipfs.

This may be due to different version of node, npm or lack of package-lock.json / yarn.lock.
Mozilla Reviewer is okay with using specific versions, as long as we provide step-by-step build instruction that result in correct file, matching the one from NPM.

Problem 2
We have 7 days to provide "sources and build steps", otherwise ipfs-companion will be removed.

@diasdavid @dignifiedquire @fbaiodias – any ideas on how to address this?

[dignifiedquire]

Created a test branch: https://github.com/ipfs/js-ipfs-api/pull/626 please test

[olizilla]

@lidel would be be allowed to submit a new version of the add-on that was built with a package-lock.json file? That way the transitive dependencies would all be pinned at specific versions as well as the direct ones. That plus a Dockerfile should be enough to get a reproducible build for future releases.

I'll submit a PR with a Dockerfile and a package-lock.json

[olizilla]

Ok so, i'm pretty sure that we're ok on this.

From the docs Example_Desired_outcome section of the code submission docs, it sounds like they want to be able to run the same commands we did to create the release, and be able to diff the output with the contents of the uploaded bundle.

i've tried running the build in docker (simulating debian jessie), and running it locally on osx, and recursively diff-ing the unzipped build artefacts reveals no differences. 🎉 I think that's all that moz want to see.

Looking at the source for a popular firefox addon, emoji-helper it looks like they are doing nothing fancier than adding a packge-lock.json. I think we should do that too, but I don't think we need to do anything urgently to get through the review process, unless moz have specific questions.

[olizilla]

@lidel i've just re-read the title of this issue... did moz already raise an issue about

provide source code + build steps that will produce exactly the same code as one published to NPM.

[lidel]

@olizilla

would be be allowed to submit a new version of the add-on that was built with a package-lock.json file?

Yes, we need to make new releases (extension + deps: js-ipfs-api and is-ipfs), as we are unable to replicate build of already submitted one.
One thing: we had mixed experiences with package-lock.json (development-wise).
I think yarn.lock and your build steps from https://github.com/ipfs/js-ipfs-api/pull/626#issuecomment-344374740 may be faster/better/safer.
Are there any cons to it?

did moz already raise an issue about (..)

Indeed, they already requested that (see https://github.com/ipfs/ipfs-companion/issues/306#issue-273607339, added original feedback there)

[lidel]

Steps are:

1. make reproducible releases of js-ipfs-api and is-ipfs -- see https://github.com/ipfs/js-ipfs-api/pull/626#issuecomment-344876722
2. switch ipfs-companion to new deps and make a new release (@lidel)
3. submit release to Mozilla with build steps from https://github.com/ipfs/js-ipfs-api/pull/626#issuecomment-344263639 + a note that on some platforms there is one line difference that can be eliminated by building in docker (https://github.com/ipfs/js-ipfs-api/pull/626#issuecomment-344374740) -- I hope this will be enough to push this version through the review. (@lidel)

So we are kinda waiting for 1)

PS. I wonder if it would not be easier to browserify entire thing (#311) instead 🤔

[olizilla]

PS. I wonder if it would not be easier to browserify entire thing (#311) instead 🤔

Strongly agree! browerify + a yarn.lock means the add-on bundle will be reproducible. The package.json in js-ipfs-api points to it's src/main.js as it's main, so browserify will bundle that rather than the index.min.js that we can't recreate.

It's still worth getting those libs builds to be reproducible, but it'd be nice to get that off the critical path to getting moz happy with ipfs-companion.

One thing: we had mixed experiences with package-lock.json (development-wise).
I think yarn.lock and your build steps from ipfs/js-ipfs-api#626 (comment) may be faster/better/safer.
Are there any cons to it?

I've not used yarn enough to say. I've not had any problems with npm, but I know that the package-lock merging dev experience needs improving. I saw this nugget from kat the other day

That file isn't really meant for human consumption. I've seen some projects mark it as a binary file which will do all the good you need.

To fix conflicts: git co package-lock.json --theirs && npm i then add and --continue.

https://twitter.com/maybekatz/status/930523154253877248

which I'm looking forward to trying.

[lidel]

Perhaps lock merging with --theirs could be run automatically run via postmerge hook (package.json)?
Are there any edge cases that would be broken with such automation?

[lidel]

Status update: 7 day window from Mozilla will end today and we need to solve it either by enabling yarn+docker in deps (https://github.com/ipfs/js-ipfs-api/pull/626) or switching extension's build to browserify (https://github.com/ipfs/ipfs-companion/pull/311).
I don't think extension will be disabled for existing users, but it won't be possible to install it by new ones.

(Worst case scenario is an emergency release with browserify that does not have working tests.)

[olizilla]

@lidel I know @alanshaw has made good progress on fixing the tests on the browserify pr. It'll be mergable today.

[lidel]

Alright, we now have two ways of making a reproducible build of build/ipfs_companion-*.zip:

With Nave+Yarn

npm install -g nave
nave use 8.9.1
npm run yarn-build

With Docker+Yarn:

npm run docker-build

I feel quite comfortable with this setup (big thanks to @olizilla for pushing this!).
There is still one line diff (caused by js-ipfs-api: https://github.com/ipfs/js-ipfs-api/pull/626#issuecomment-344349608) but it should not be a problem, and will be fixed upstream eventually.

Let's keep this issue open until first "reproducible release" is reviewed and accepted at Mozilla.

[lidel]

I've just submitted v2.1.0, fingers crossed 🤞

[daviddias]

This is done ✅, right? Are we missing something?

[daviddias]

Some really cool stats!! :D

[lidel]

This is done ✅, right? Are we missing something?

I think it is done for now, yarn-based release build seems to not raise any red flags
(sadly there is no indicator at the AMO site, but it's been a week, so.. 🙃 ).