search

then / promise

Bare bones Promises/A+ implementation
https://www.promisejs.org
MIT License
2.58k stars 312 forks source link

Make the build reproducible #148

Closed lamby closed 6 years ago

lamby commented 6 years ago

Whilst working on the Reproducible Builds effort [0], we noticed that node-promise could not be built reproducibly as it uses random numbers as throwaway identifiers.

This patch uses a determinstic suffix for these identifiers based on the contents of the src/ directory.

This was filed in @Debian as https://bugs.debian.org/886277. There was also a previous aborted attempt in #146.

[0] https://reproducible-builds.org/ [1] https://github.com/then/promise/pull/146

Signed-off-by: Chris Lamb chris@chris-lamb.co.uk

lamby commented 6 years ago

I'm not entirely sure what happened in #146, but trying again here assuming good faith all round :)

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 95.783% when pulling c62e840dabac4d8fd7fa521baf664c3ff278f4b0 on lamby:reproducible-build-2 into cf30e9f39476bbfa3dbaacb93127c1c958d5f255 on then:master.

kpcyrd commented 6 years ago

@lamby fs.readdirSync uses readdir(3) underneath which has non-deterministic ordering according to the documentation:

       The order in which filenames are read by successive calls to
       readdir() depends on the filesystem implementation; it is unlikely
       that the names will be sorted in any fashion.

I think this list needs to be sorted explicitly.

lamby commented 6 years ago

I think this list needs to be sorted explicitly.

Thanks, updated. (Curiously, when I was trying the ++idx patch I was running under disorderfs and, as I wasn't seeing any variation, I thus assumed readdirSync was sorted.)

lamby commented 6 years ago

Hey folks, any update on this?

ForbesLindesay commented 6 years ago

@lamby The ball is in your court. I've explained what I'd like to see in order to merge this:

  1. Single character identifier names where possible
  2. Zero chance of collision in generated names (simple to achieve using something like the code I posted)
lamby commented 6 years ago

Thanks @ForbesLindesay, I must have missed that comment. Updated patch pushed, rebased against master. Thanks o/