Replace instance variable access with call to .elf.

[rmNULL]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

.elf was introduced in gem release v1.1.0.
Resolves #7678 (comment)

[rmNULL]

cc @sjackman @woodruffw

[jonchang]

We don’t have version requirements for the other gems. Should we update Gemfile.lock instead?

[woodruffw]

We don’t have version requirements for the other gems. Should we update Gemfile.lock instead?

Perhaps both? We won't be able to function with versions of patchelf.rb below 1.1 from here on out, so it makes sense to me to enforce it in the Gemfile itself and also to check-in the relevant lockfile changes.

@rmNULL, could you do that?

[jonchang]

Perhaps both? We won't be able to function with versions of patchelf.rb below 1.1 from here on out, so it makes sense to me to enforce it in the Gemfile itself and also to check-in the relevant lockfile changes.

Yeah, but it should always update to the latest in the lockfile, since setting HOMEBREW_PATCHELF_RB will always call install_bundler_gems!, which will always install the version specified in the lockfile.

I note that patchelf isn't currently specified in Gemfile.lock, so this means that it'll always be up to date assuming it doesn't conflict with other things in the lockfile.

[sjackman]

For consistency with the rest of the Gemfile I agree with removing the version dependency.

[woodruffw]

Yeah, but it should always update to the latest in the lockfile, since setting HOMEBREW_PATCHELF_RB will always call install_bundler_gems!, which will always install the version specified in the lockfile.

I note that patchelf isn't currently specified in Gemfile.lock, so this means that it'll always be up to date assuming it doesn't conflict with other things in the lockfile.

Good points. No objections here!

[jonchang]

If anything, we should:

• remove the ENV variable test in Gemfile
• commit patchelf to Gemfile.lock

This is fine because (and correct me if I'm wrong) we always check for HOMEBREW_PATCHELF_RB and call install_vendor_gems! on all codepaths that would use patchelf.rb. This means that we no longer get dirty work trees when HOMEBREW_PATCHELF_RB is set, and anyone actually using the functionality will have the right gems installed on demand.

When we flip the switch so everyone uses patchelf.rb, we can use brew vendor-gems and commit patchelf.rb under vendor/.

[woodruffw]

This is fine because (and correct me if I'm wrong) we always check for HOMEBREW_PATCHELF_RB and call install_vendor_gems! on all codepaths that would use patchelf.rb. This means that we no longer get dirty work trees when HOMEBREW_PATCHELF_RB is set, and anyone actually using the functionality will have the right gems installed on demand.

When we flip the switch so everyone uses patchelf.rb, we can use brew vendor-gems and commit patchelf.rb under vendor/.

Yeah, this sounds preferable to me. I can't recall why we didn't do this in the first place, other than maybe not wanting to install patchelf.rb unconditionally on all Homebrew boxes. But we'll be doing that when we vendor eventually, so I don't think it's a huge deal.

[jonchang]

Now that HOMEBREW_DEVELOPER implies HOMEBREW_PATCHELF_RB there's no reason to avoid committing to the lockfile anymore. But prior to that change I think it made sense.

[Bo98]

Now that HOMEBREW_DEVELOPER implies HOMEBREW_PATCHELF_RB there's no reason to avoid committing to the lockfile anymore. But prior to that change I think it made sense.

Would it affect Dependabot at all?

[jonchang]

Would it affect Dependabot at all?

I think dependabot would push updates to the lockfile as usual.

[Bo98]

I asked since Dependabot can purge dependencies no longer used, and I wasn't sure how it did that internally.

[jonchang]

I think dependabot basically does bundle update foo on the lockfile, so if we make the Gemfile more "standard" (not changing behavior based on environment variables) it'll make things easier for everyone, including the bots :)

[Bo98]

make the Gemfile more "standard" (not changing behavior based on environment variables)

👍 on this.

I missed that point earlier - I thought this was about committing the lockfile only.

[sjackman]

Yeah, this sounds preferable to me. I can't recall why we didn't do this in the first place, other than maybe not wanting to install patchelf.rb unconditionally on all Homebrew boxes.

The conditional Gemfile was at Mike's request, and yes, that was the reasoning.

[sjackman]

LGTM!