Linuxbrew / brew

:beer::penguin: The Homebrew package manager for Linux
https://linuxbrew.sh
BSD 2-Clause "Simplified" License
2.66k stars 237 forks source link

ENV: pass formula to setup_build_environment #785

Closed iMichka closed 6 years ago

iMichka commented 6 years ago

Second attempt to fix the perl postinstall step.

The perl formula will need to be modified and will need to pass self to with_build_environment.

codecov-io commented 6 years ago

Codecov Report

Merging #785 into master will decrease coverage by 0.01%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #785      +/-   ##
==========================================
- Coverage   70.43%   70.42%   -0.02%     
==========================================
  Files         384      384              
  Lines       20632    20632              
==========================================
- Hits        14533    14530       -3     
- Misses       6099     6102       +3
Impacted Files Coverage Δ
utils/analytics.rb 82.05% <0%> (-7.7%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 199540b...6c47d2e. Read the comment docs.

jonchang commented 6 years ago

I guess I'm a bit lost, there only seems to be one other caller for this function and no callers other than perl.rb in linuxbrew. I guess it's fine but I'm not sure if upstream would be interested in this patch since the perl postinstall step seems like almost a hack.

iMichka commented 6 years ago

This is the point. I guess that the we do not want to have superenv enabled during postinstall by default. So you need a way to manually set the env. For Perl we need to compile these modules in the postinstall step (not sure why exactly).

Soni think this here will fix our current problem. If it is what we want in the long run as a solution is a different topic.

sjackman commented 6 years ago

We recently enabled ENV.refurbish_args by default. brew postinstall does not call ENV.setup_build_environment perl calls ENV.with_build_environment in postinstall ENV.with_build_environment calls ENV.setup_build_environment(formula = nil) HOMEBREW_FORMULA_PREFIX is set to nil shims/super/cc does not know which formula is being built. These two lines of code do not work: https://github.com/Linuxbrew/brew/blob/199540b52f6e33184a061eda8473042a98e0ad03/Library/Homebrew/shims/super/cc#L231-L232

Instead of Perl calling ENV.with_build_environment, I propose we instead call…

ENV.setup_build_environment(self)
sjackman commented 6 years ago

Closing without merging. See above comment.

sjackman commented 6 years ago

Superseded by PR https://github.com/Linuxbrew/homebrew-core/pull/8842