Skip to content

Conversation

@dcarley
Copy link
Contributor

@dcarley dcarley commented Jun 18, 2025

⚠️ Depends on fixes in flox/flox#3257 but would like a review in the meantime.

Verified with #16 and:

% FLOXBIN="nix run github:flox/flox/brantley/remove-layered-activation-from-manifest-build --" make ruby         
./test.sh quotes-app-rb
…
📋 Test Summary:
  - ✅ quotes-app-rb
  - ✅ quotes-app-rb-pure

ruby: Fix pure build

By splitting the dependency fetching into an impure build and
referencing the cache directory from the pure build.

This avoids using or modifying unversioned config and cache from the
current working directory so that the build is as reproducible as
possible.

It took a while to find the right Bundler options to achieve this from
these docs:

Even so, the bundle cache --no-install command appears to always copy
the gems twice, so I'm manually deleting the non-cache version.

I'm unsure how much of this was hard because the project structure
doesn't match what you'd get from bundle gem <name>, which create its
own binstub and gemspec, but I don't want to make too many changes at
once.

ruby: More targeted copy in pure build

Only copy what we need rather than copying everything and then deleting
things that we don't expect. This prevents a vendor directory from
being added to the package twice and any result- symlinks.

ruby: Make non-pure look more like deps+pure

Backport the config options, install commands, and targeted copy from
the deps and pure builds in the two preceding commits.

@dcarley dcarley force-pushed the dcarley/ruby-pure branch from 0d5d6e1 to 06ac4af Compare June 19, 2025 10:33
@dcarley dcarley marked this pull request as ready for review June 19, 2025 10:36
@dcarley dcarley requested a review from a team June 19, 2025 10:36
Copy link
Contributor

@ysndr ysndr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont know much about ruby but i dont see anything off with the approach itself

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didnt we started to write a gitattributes file to suppress lockfile changes in these diffs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These environments were created (6332101 on Apr 16th) before that change was released (v1.4.3 on May 28th).

dcarley added 3 commits June 23, 2025 10:46
By splitting the dependency fetching into an impure build and
referencing the cache directory from the pure build.

This avoids using or modifying unversioned config and cache from the
current working directory so that the build is as reproducible as
possible.

It took a while to find the right Bundler options to achieve this from
these docs:

- https://bundler.io/v2.5/man/bundle-config.1.html
- https://bundler.io/v2.5/man/bundle-cache.1.html
- https://bundler.io/v2.5/man/bundle-install.1.html

Even so, the `bundle cache --no-install` command appears to always copy
the gems twice, so I'm manually deleting the non-cache version.

I'm unsure how much of this was hard because the project structure
doesn't match what you'd get from `bundle gem <name>`, which create its
own binstub and gemspec, but I don't want to make too many changes at
once.
Only copy what we need rather than copying everything and then deleting
things that we don't expect. This prevents a `vendor` directory from
being added to the package twice and any `result-` symlinks.
Backport the config options, install commands, and targeted copy from
the deps and pure builds in the two preceding commits.
@dcarley dcarley force-pushed the dcarley/ruby-pure branch from 06ac4af to d993ae5 Compare June 23, 2025 09:46
@dcarley dcarley merged commit 649e00f into main Jun 23, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants