beamPackages.buildMix: compile non-deterministic deps in $out/src

[ju1m]

This is necessary to support mix compile.surface or Fine.include_dir().

Used by #550073

Default erlangDeterministicBuilds to false
because it breaks apps using Surface
and does not improve determinism within a sandboxed nix build which always builds in the same path anyway.
EDIT: keep erlangDeterministicBuilds to true by default because it apparently increases the closure size.

Yet, disabling erlangDeterministicBuilds is not enough for mix compile.surface to work across packages, as it:

• needs to access the content of the source path of the dependencies,
• and, uses as source path the actual path where the dependencies were built.

Since beamCopySourceHook only copies $src into $out/src, that is not enough: module_info/1 will return a path in /build/source instead of a path in $out/src,
therefore build into $out/src, which will remain reachable after the build.

Issue: surface-ui/surface#762

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

[adamcstephens]

Why is it necessary to change the global default for this? Can't surface, or other packages, disable deterministic individually?

[ju1m]

@adamcstephens
My goal in this PR is to make a correct package the easiest thing to come up with.
My rationale is that nix's sandbox must be enabled
when one cares about determinism/reproducibility.
and erlangDeterministicBuilds == true is useless when using nix's sandbox:

deterministic - Omit the options and source tuples in the list returned by Module:module_info(compile), and reduce the paths in stack traces to the module name alone. This option will make it easier to achieve reproducible builds.

Source: https://www.erlang.org/doc/apps/compiler/compile.html

From my somewhat limited experience in packaging Elixir in Nix,
I don't know of any case where a Nix packager would need erlangDeterministicBuilds == true.
But, I do know first hand that erlangDeterministicBuilds == true as default subtly (and sometimes silently) breaks all packages relying on module_info(), laying a trap to nix packager who like me may have to spend a lot of debugging time to understand what's wrong in nix whereas it works fine out of nix.

That's why I propose to stick to Erlang's default, which is to not enable ERL_COMPILER_OPTIONS=deterministic by default, which becomes important with that PR now exposing the correct $out/src (instead of /build/source) in module_info().

[adamcstephens]

This is going to need a full nixpkgs-review, and probably build some of the nixos tests for elixir apps in tree. Furthermore I’m not comfortable merging this until at least after branch off for 26.05, to avoid unexpected consequences. I’d also be worried that this will leave a bunch of extra artifacts in the output.

[Munksgaard]

As I remember, quite a bit of effort was put into enabling erlangDeterministicBuilds in #271288. I don't understand the full impact of the proposed changes, but we should probably loop in @happysalada, @scvalex and @mattpolzin.

cc @NixOS/beam

[mattpolzin]

I don't know of any case where a Nix packager would need erlangDeterministicBuilds == true

The short answer is that it's about not bloating the resulting derivation with unnecessary runtime dependencies.

Take a look at the PR that introduced erlangDeterministicBuilds and the history it links to if you'd like a longer answer to your question: #271288

[adamcstephens]

The short answer is that it's about not bloating the resulting derivation with unnecessary runtime dependencies.

This is the more correct way of stating what I hinted at in my comment about extra artifacts. I can see this being a real problem. For example a mixRelease package generally bundles erts in the package itself. If its closure also pulled in erlang and its dependencies, that could be 300-1300MB+ of extra unnecessary dependencies.

As I've reflected on this more, we shouldn't change the default on deterministic builds, but we could improve the support for those who want to disable them. That means potentially a modification to a hook, or an extra hook, that will ensure that we get a proper output when they're disabled.

[ju1m]

• Keep erlangDeterministicBuilds's default to false

[adamcstephens]

I would like to see us change sourceRoot only in the case of non-deterministicBuilds. mix doesn't necessarily keep the source directory clean of artifacts, especially when working with native code.

I expect src to be a clean copy of the source itself, without any build artifacts that mix may create, so I'd also consider it more correct for non-deterministicBuilds to delete and re-copy the original src after building is done. This would provide the same clean slate as the deterministicBuilds scenario. This is less of a blocker for me, but from what I've seen you probably don't want the artifacts mix leaves around.

[ju1m]

Turned this PR into a draft and pushed some changes to address latest concern raised, but cannot test yet.

• Register old beamCopySourceHook or new beamCopySourceToSourceRootHook depending on erlangDeterministicBuilds
• Keep ./config writable because some package write to it (eg. surface_form_helpers).

[adamcstephens]

surface writes to config?!? could they, not? that seems like a bad practice that we're enabling instead of pushing back on.

[ju1m]

surface writes to config?!? could they, not? that seems like a bad practice that we're enabling instead of pushing back on.

I don't know but the way Bonfire (not in Nixpkgs, yet) uses it,
surface_form_helpers tries to remove and generate .hooks.js files in appConfigPath:

$ nix -L build -f. bonfire.ember.mixNixDeps.surface_form_helpers
[…]
erlang27.3.4.7-surface_form_helpers> Generated surface_form_helpers app
erlang27.3.4.7-surface_form_helpers> <- Ran mix compile.app in 4ms
erlang27.3.4.7-surface_form_helpers> -> Running mix compile.surface --no-deps-check (inside Surface.Form.MixProject)
erlang27.3.4.7-surface_form_helpers> ** (File.Error) could not remove file "/nix/store/7ya12s6c62v1kq3bv18skii86lwrcg6w-erlang27.3.4.7-surface_form_helpers-0.2.0/src/config/current_flavour/assets/hooks/Bonfire.Editor.Milkdown.hooks.js": permission denied
erlang27.3.4.7-surface_form_helpers>     (elixir 1.18.4) lib/file.ex:1222: File.rm!/1
erlang27.3.4.7-surface_form_helpers>     (elixir 1.18.4) lib/enum.ex:987: Enum."-each/2-lists^foreach/1-0-"/2
erlang27.3.4.7-surface_form_helpers>     (surface 0.12.3) lib/mix/tasks/compile/surface.asset_generator.ex:312: Mix.Tasks.Compile.Surface.AssetGenerator.delete_unused_hooks_files!/2
erlang27.3.4.7-surface_form_helpers>     (surface 0.12.3) lib/mix/tasks/compile/surface.asset_generator.ex:191: Mix.Tasks.Compile.Surface.AssetGenerator.generate_js_files/2
erlang27.3.4.7-surface_form_helpers>     (surface 0.12.3) lib/mix/tasks/compile/surface.asset_generator.ex:35: Mix.Tasks.Compile.Surface.AssetGenerator.do_run/2
erlang27.3.4.7-surface_form_helpers>     (surface 0.12.3) lib/mix/tasks/compile/surface.ex:198: Mix.Tasks.Compile.Surface.do_run/1
erlang27.3.4.7-surface_form_helpers>     (mix 1.18.4) lib/mix/task.ex:495: anonymous fn/3 in Mix.Task.run_task/5
erlang27.3.4.7-surface_form_helpers>     (stdlib 6.2.2.2) timer.erl:595: :timer.tc/2

I can gate that behind a appConfigPathWritable if you prefer?

[adamcstephens]

Are you sure that’s not a misconfiguration or maybe can be configured to write elsewhere? I wouldn’t expect assets, js or really any content to generate into config.

[adamcstephens]

https://surface.hexdocs.pm/Mix.Tasks.Compile.Surface.html#module-options

hooks_output_dir is the config key that changes this.

Why it's trying to delete project files is a bit unclear to me.

Can you share your build target? Is it https://github.com/bonfire-networks/bonfire-app on main?

[ju1m]

@adamcstephens I'm currently packaging Bonfire in NGIpkgs here: ngi-nix/ngipkgs#2155
See profiles/pkgs/development/beam-modules/build-mix/mix-app-config-patch-hook.sh to uncomment cp and comment ln:

    #cp --no-preserve=mode -r "${appConfigPath}" config
    ln -sf "${appConfigPath}" config

But do no expect it to work besides bonfire.ember.mixNixDeps.surface_form_helpers, I'm still struggling to make mixNixDeps.exla and mixNixDeps.evision updates work.
NGIpkgs has been archived, hence this PR to prepare a possible migration of Bonfire to Nixpkgs.

[adamcstephens]

Have you considered using mixFodDeps instead? I wonder if this would make your life easier, at the cost of rebuild efficiency.

[ju1m]

Have you considered using mixFodDeps instead? I wonder if this would make your life easier, at the cost of rebuild efficiency.

Ah, yes, it would, mixFodDeps is what was used by previous attempts to package Bonfire:
https://github.com/bonfire-networks/bonfire-app/blob/03c6cc1ddbec1dfe99a7967c9444cb7bdb040b73/nix/package.nix#L16-L33

But I've been struggling/able to avoid it up to now, because it undermines the added value of using nix wrt. Docker
to precisely blame reproducibility problems and build from source instead of pulling dozens of binaryNativeCode
(C and Rust NIFs, xla, opencv, …),
sometimes not even free software without upstream developers being aware.

[adamcstephens]

I've taken a stab at a new PR title. Can you update the commit to something similar?