Reproducible builds on Linux

[sxa]

In the dim and distant past before global lockdowns there was some previous work to make the build process binary reproducible.

I had a play recently with the process (on Linux/aarch64 because they are the quickest machines I have access to) and I hit two issues:

1. The production of debug-support.cc is nondeterministic - the definitions are not always in the same order within the file. It is generated by https://github.com/nodejs/node/blob/main/deps/v8/tools/gen-postmortem-metadata.py - If I copy in a consistent one the builds can be reproducible.
2. I had to disable the snapshot functionality (configure --without-node-snapshot), which suggests that the changes made in node_code_cache.cc and node_snapshot.cc generation is unreproducible node#29108 are no longer valid for the current release.

Tagging @ChALkeR who put the original PR in (albeit three years ago!)

[mhdawson]

@joyeecheung do you have any thoughts/suggestions on the snapshot front?

@sxa I assume that it should be possible to update https://github.com/nodejs/node/blob/main/deps/v8/tools/gen-postmortem-metadata.py so that it generates data in a deterministic manner (maybe sorting them at some point) and it's really just a matter of doing that versus there being some fundamental reason whey it might not be possible?

[sxa]

That would be my assumption yes. The previous issues and PRs suggest this was resolved, so we'd just need to understand what happened and potentially reimplement a fix for it.

[rvagg]

Pinging @warpfork for this thread. He's currently heads-down on building Warpforge and has a passion for reproducible builds and I was telling him that there's interest in making progress on this front for Node.js so there might be some potential for collaboration and the production of some tooling to both generate reproducible builds and also document & provide mechanisms for people to do it for themselves without all the pain that usually comes from doing it manually.

So @sxa, meet the most excellent and clever @warpfork; maybe you two should chat.

[sxa]

Thanks @rvagg - I've worked on reproducible build environments for another project, and the Node.js build is close other than those two issues above, so it should just require a little bit of knowledge about the Node processes involved in producing those two things to be able to resolve it, then we can possibly put some more robust things in place for making our builds reproducible by default, which I think should be the aim here.

@warpfork DO you have much experience on non-Linux reproducible build work (I've so far only looked at Linux for Node.js)

[StefanBruens]

1. The production of `debug-support.cc` is nondeterministic - the definitions are not always in the same order within the file. It is generated by https://github.com/nodejs/node/blob/main/deps/v8/tools/gen-postmortem-metadata.py - If I copy in a consistent one the builds can be reproducible.

One source of indeterminism is here:

https://github.com/nodejs/node/blob/ab064d12b79d14a3d02ba420138cc9d24169a951/deps/v8/tools/gen-postmortem-metadata.py#L717

Instead of a set a dict can be used here (with None values), as since Python 3.7 dict preserves order. Then use for line in dict.fromkeys(lines): to output the lines in order.

[cclauss]

Is there a todo on this issue or can it be closed?

[sxa]

Definitely still work to do and I still plan to look at it.

[sxa]

Just tried four consecutive builds with the latest code and the debug-support.cc came out the same each time. There have been four V8 version bumps since I last tested (10.1.124.6 ->11.3.244.4) so either I'm lucky today or something in there has made things better, but potentially the fix in the earlier comment will no longer required.

This was tested with ./configure --without-node-snapshot and setting SOURCE_DATE_EPOCH=0 in the environment.

With snapshots enabled the builds are still non-reproducible. due to node_snapshot.cc (Note that node_code_cache.cc referred to in nodejs/node#29108 is no longer a problem on the Linux system being used.

[sxa]

Looks like the break was between these two commits, so nodejs/node@1faf6f459f likely made it non-reproducible again.

* 1faf6f459f 2019-04-21 | src: snapshot Environment upon instantiation (HEAD, refs/bisect/bad) [Joyee Cheung]
* f04538761f 2020-04-30 | tools: enable Node.js command line flags in node_mksnapshot (refs/bisect/good-f04538761f5bb3c334d3c8d16d093ac0916ff3bc) [Joyee Cheung]

FYI @joyeecheung (PR) @bnoordhuis (PR)

I don't think I have enough knowledge of this code to be able to propose a safe solution here so looking for advice.

[sxa]

@joyeecheung Would you be able to assist with the reproducibility of the snapshots now? I'm not sure who else might have good knowledge of the snapshot support in Node so anyone else might have to start from scratch.

[joyeecheung]

Thanks for the ping, not sure how I missed the earlier one. I diff'ed the generated snapshot locally and found ~20 lines of differences in the snapshot.cc generated (with --predictable, which we should also set for mksnapshot), most notably the binding data are initialized in different order, not sure why but they are supposed to be initialized in the same order. I'll look into why that happens.

[joyeecheung]

With this patch the snapshot.cc difference is down to 13 lines. Still need to figure out the differences in the blob though.

see diff

diff --git a/src/cleanup_queue-inl.h b/src/cleanup_queue-inl.h
index 5d9a56e6b0..d1fbd8241d 100644
--- a/src/cleanup_queue-inl.h
+++ b/src/cleanup_queue-inl.h
@@ -39,7 +39,9 @@ void CleanupQueue::Remove(Callback cb, void* arg) {
 
 template <typename T>
 void CleanupQueue::ForEachBaseObject(T&& iterator) const {
-  for (const auto& hook : cleanup_hooks_) {
+  std::vector<CleanupHookCallback> callbacks = GetOrdered();
+
+  for (const auto& hook : callbacks) {
     BaseObject* obj = GetBaseObject(hook);
     if (obj != nullptr) iterator(obj);
   }
diff --git a/src/cleanup_queue.cc b/src/cleanup_queue.cc
index 6290b6796c..c0fcda2fac 100644
--- a/src/cleanup_queue.cc
+++ b/src/cleanup_queue.cc
@@ -5,7 +5,7 @@
 
 namespace node {
 
-void CleanupQueue::Drain() {
+std::vector<CleanupQueue::CleanupHookCallback> CleanupQueue::GetOrdered() const {
   // Copy into a vector, since we can't sort an unordered_set in-place.
   std::vector<CleanupHookCallback> callbacks(cleanup_hooks_.begin(),
                                              cleanup_hooks_.end());
@@ -20,6 +20,12 @@ void CleanupQueue::Drain() {
               return a.insertion_order_counter_ > b.insertion_order_counter_;
             });
 
+  return callbacks;
+}
+
+void CleanupQueue::Drain() {
+  std::vector<CleanupHookCallback> callbacks = GetOrdered();
+
   for (const CleanupHookCallback& cb : callbacks) {
     if (cleanup_hooks_.count(cb) == 0) {
       // This hook was removed from the `cleanup_hooks_` set during another
diff --git a/src/cleanup_queue.h b/src/cleanup_queue.h
index 64e04e1856..2ca333aca8 100644
--- a/src/cleanup_queue.h
+++ b/src/cleanup_queue.h
@@ -6,6 +6,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <unordered_set>
+#include <vector>
 
 #include "memory_tracker.h"
 
@@ -66,6 +67,7 @@ class CleanupQueue : public MemoryRetainer {
     uint64_t insertion_order_counter_;
   };
 
+  std::vector<CleanupHookCallback> GetOrdered() const;
   inline BaseObject* GetBaseObject(const CleanupHookCallback& callback) const;
 
   // Use an unordered_set, so that we have efficient insertion and removal.
diff --git a/tools/snapshot/node_mksnapshot.cc b/tools/snapshot/node_mksnapshot.cc
index ecc295acdb..2ba6878a28 100644
--- a/tools/snapshot/node_mksnapshot.cc
+++ b/tools/snapshot/node_mksnapshot.cc
@@ -52,6 +52,7 @@ int main(int argc, char* argv[]) {
 #endif  // _WIN32
 
   v8::V8::SetFlagsFromString("--random_seed=42");
+  v8::V8::SetFlagsFromString("--predictable");
   v8::V8::SetFlagsFromString("--harmony-import-assertions");
   return BuildSnapshot(argc, argv);
 }

[sxa]

Sounds like good progress - thanks @joyeecheung!