Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

[WIP] Reproducible builds #626

Closed
wants to merge 1 commit into from
Closed

[WIP] Reproducible builds #626

wants to merge 1 commit into from

Conversation

dignifiedquire
Copy link
Contributor

Ref ipfs/ipfs-companion#306

@lidel @diasdavid can you please checkout this branch and try building with the following set of deps

  • node@8.9.1
  • yarn@1.2.1

This should produce the same builds as stored in dist-locked.
The checksums should be

> md5 dist-locked/*
MD5 (dist-locked/index.js) = 20105d3170c84e790ae27d2fb8ad870f
MD5 (dist-locked/index.js.map) = df9fdb2676517ec6775f9e0ac1f5d0a4
MD5 (dist-locked/index.min.js) = faa4541a664160a6ec9d7243b7635147

@olizilla
Copy link
Contributor

@dignifiedquire
A quick data point for you. I ran:

nave use 8.9.1
npx yarn@1.2.1
npx yarn@1.2.1 build

and got

$ md5 dist/*
MD5 (dist/index.js) = 20105d3170c84e790ae27d2fb8ad870f
MD5 (dist/index.js.map) = da28e9e4c417245dd029685325765a64
MD5 (dist/index.min.js) = faa4541a664160a6ec9d7243b7635147

Notably the source map dist/index.js.map differs.

@dignifiedquire
Copy link
Contributor Author

Thanks @olizilla what OS are you using?
I think if we get the source files to be reproducible and only the map files we are probably safe. Though I would love to know why those are different...

@olizilla
Copy link
Contributor

I'm on osx 10.12.6

@lidel
Copy link
Contributor

lidel commented Nov 14, 2017

I've run:

> npm install -g nave
> nave use 8.9.1
> npx yarn@1.2.1
> npx yarn@1.2.1 build

and got different checksums under GNU/Linux (--tag provides BSD-style output):

> md5sum --tag dist-locked/*
MD5 (dist-locked/index.js) = 20105d3170c84e790ae27d2fb8ad870f
MD5 (dist-locked/index.js.map) = df9fdb2676517ec6775f9e0ac1f5d0a4
MD5 (dist-locked/index.min.js) = faa4541a664160a6ec9d7243b7635147

> md5sum --tag dist/*
MD5 (dist/index.js) = fdbff9ad66965fd08689bd7c115ef39f
MD5 (dist/index.js.map) = 6d8c1978157a8c315526ba47b164f2ba
MD5 (dist/index.min.js) = 3586befa15d897988ce2895eda0a5fd0

Non-minified differences are very small tho: diff dist-locked/index.js dist/index.js returns 1 line:

52904c52904
<         isWebkit = !_crypto.subtle && !!_crypto.webkitSubtle;
---
>         isWebkit = !!_crypto.webkitSubtle;

May be something on my end. Is there anything I could try?

@victorb
Copy link
Contributor

victorb commented Nov 14, 2017

Getting the same as @lidel with Ubuntu 16.04.3 LTS

➜  js-ipfs-api git:(lock-down) $ nvm install 8.9.1
v8.9.1 is already installed.
Now using node v8.9.1 (npm v5.5.1)

➜  js-ipfs-api git:(lock-down) $ npx yarn@1.2.1
npx: installed 1 in 0.705s
yarn install v1.2.1
[1/5] Validating package.json...
[2/5] Resolving packages...
success Already up-to-date.
Done in 0.63s.

➜  js-ipfs-api git:(lock-down) $ npx yarn@1.2.1 build
npx: installed 1 in 0.77s
yarn run v1.2.1
$ aegir build
 ✔ Clean ./dist
 ✔ Webpack Build (1.8 MB)
 ✔ Minify (873.48 KB)
Done in 9.87s.

➜  js-ipfs-api git:(lock-down) $ lsb_release -a | grep Description
Description:    Ubuntu 16.04.3 LTS

➜  js-ipfs-api git:(lock-down) $ md5sum --tag dist-locked/*
MD5 (dist-locked/index.js) = 20105d3170c84e790ae27d2fb8ad870f
MD5 (dist-locked/index.js.map) = df9fdb2676517ec6775f9e0ac1f5d0a4
MD5 (dist-locked/index.min.js) = faa4541a664160a6ec9d7243b7635147

➜  js-ipfs-api git:(lock-down) $ md5sum --tag dist/*
MD5 (dist/index.js) = fdbff9ad66965fd08689bd7c115ef39f
MD5 (dist/index.js.map) = 42fce7851648a2f27b7ea0703e9790d9
MD5 (dist/index.min.js) = 3586befa15d897988ce2895eda0a5fd0

@dignifiedquire
Copy link
Contributor Author

greeeeaaat...

@dignifiedquire
Copy link
Contributor Author

@lidel can we require them to use osx?

@olizilla
Copy link
Contributor

super weird. I rm -rf'd node_modules and dist and just tried it again in docker, like:

FROM node:8.9.1
WORKDIR /src
COPY . /src
RUN npx yarn@1.2.1
RUN npx yarn@1.2.1 build
RUN md5sum --tag dist/*
$ docker build -t lock-down .
# much docker

Step 6/6 : RUN md5sum --tag dist/*
 ---> Running in 2e8aac3fff22
MD5 (dist/index.js) = 20105d3170c84e790ae27d2fb8ad870f
MD5 (dist/index.js.map) = b8199ce59a5650f6fcd49c439cbb6d6d
MD5 (dist/index.min.js) = faa4541a664160a6ec9d7243b7635147

which differs on the source map again, but the source files match.

@lidel
Copy link
Contributor

lidel commented Nov 15, 2017

@dignifiedquire probably not:

Tools you are using to obfuscate, minify or concatenate your source code must be open source, as otherwise we cannot verify the operation of the tool.
https://developer.mozilla.org/en-US/Add-ons/Source_Code_Submission

@olizilla I used your Dockerfile and got exactly the same checksums:

Step 6/6 : RUN md5sum --tag dist/*
 ---> Running in f4c963c7fdae
MD5 (dist/index.js) = 20105d3170c84e790ae27d2fb8ad870f
MD5 (dist/index.js.map) = b8199ce59a5650f6fcd49c439cbb6d6d
MD5 (dist/index.min.js) = faa4541a664160a6ec9d7243b7635147

We are getting somewhere.

@olizilla
Copy link
Contributor

@dignifiedquire do you have a plan in mind for next steps here? I'm happy to help out.

@dignifiedquire
Copy link
Contributor Author

dignifiedquire commented Nov 16, 2017 via email

@lidel
Copy link
Contributor

lidel commented Nov 19, 2017

Quick question: is it possible to do the first step (create NPM releases of js-ipfs-api and is-ipfs with yarn lock).. today?

I ask because 7 day window for ipfs/ipfs-companion#306 will end today and I will have to solve it somehow – want to plan accordingly.

@daviddias
Copy link
Contributor

daviddias commented Nov 20, 2017

@lidel @olizilla Ack. If I understand correctly, the issue is that we cannot absolutely avoid having reproducible builds for ipfs-companion and now we are just figuring out how to release the reproducible build.

On:

image

We won't be able to point to a bundled vesion. Is js:next a thing now? Can we have another key in the package.json that Bundlers like Webpack and Browserify will pick up? This would also solve things like #611

On:

image

This sounds good. We can have a yarn release. Does this fulfill all the criteria?

Let's get this done ASAP. Ping me on IRC.

@olizilla
Copy link
Contributor

olizilla commented Nov 20, 2017

I think there are a few related issues here...

  1. Can we get a reproducible build for ipfs-companion?

I believe that is solved by adding a browserify build and a yarn.lock to ipfs-companion.

  1. Can we get a reproducible build of ipfs-js*?

That should be solved by adding a yarn.lock here.

  1. Can we get a browser / ES5 friendly bundle published to npm

Probably, using thebrowser key in package.json and running babel pre-publish as per that PR as per #630

@atfornes
Copy link
Contributor

@olizilla, do you mean this PR: #630?

I can change it with your proposal, adding a "browser": "lib/index.js" to package.json instead of changing its "main"property.

Please note that it is important to use "prepare" npm script instead of "prepublish" so the builds are triggered for all installations and not only for official releases: https://stackoverflow.com/a/44680575/4928558

@olizilla
Copy link
Contributor

@atfornes yes, #630. I've updated my comment to better express that.

Right now the focus is on getting a reproducible build of ipfs-companion. I think #630 is reasonable, I'll ping you about it next week when I have some time available, if that's ok.

@olizilla
Copy link
Contributor

@lidel @dignifiedquire we spoke to @diasdavid and the plan right now is to get ipfs/ipfs-companion#311 mergeable, which makes the ipfs-companion build reproducible without requiring a change to js-ipfs-api.

We don't need to solve the reproducibility of js-ipfs-api browser bundles today, but it's a good property to have, and we'll be working on that and the related issue of transpilation of language features vs shimming of modules that don't make sense in the browser.

@daviddias
Copy link
Contributor

@lidel @olizilla @alanshaw, can I get a greenlight to close this PR as the reproducible builds thing was solved at the ipfs-companion level?

@lidel
Copy link
Contributor

lidel commented Nov 22, 2017

@diasdavid there is a remaining issue of 1-line-diff (between macOS and Linux) mentioned in #626 (comment).
I guess if someone knowledgeable with the matter opens an upstream ticket (for webpack?), we can close this one.

@olizilla
Copy link
Contributor

@diasdavid Yep, let's close this and come up with a plan for reproducible builds next week.

@daviddias daviddias closed this Nov 22, 2017
@daviddias daviddias deleted the lock-down branch November 22, 2017 09:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants