ENV: move to new paths. #507

MikeMcQuaid · 2016-07-13T17:11:59Z

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 tests with your changes locally?

Move some stuff formerly in Library/ENV around:

Move Library/ENV/$XCODE_VERSION to Library/Homebrew/env/super as they are
all superenv wrappers and all symlinks to the same version. We never needed
the "separate shims for separate versions" functionality and it just adds
confusion.
Move Library/ENV/pkgconfig to Library/Homebrew/env/pkgconfig to get more
things under Library/Homebrew
Move Library/ENV/scm to Library/scm as these wrappers are not actually
used by or related to superenv (or stdenv) in any way.

xu-cheng · 2016-07-13T17:29:42Z

I briefly checked all scripts, ~~it seems that all of them should work fine although some of them make some assumption on file location.~~ Double checked, the git/svn script in the superenv will fail to locate brewed git/svn even after fixing the symlink.

But git and svn symlink in superenv will have to be updated accordingly.

MikeMcQuaid · 2016-07-14T07:55:53Z

@xu-cheng Thanks, fixed.

xu-cheng · 2016-07-14T08:02:57Z

You fixed super/git script but broke scm/git in the process.

To check it, you can add echo $brew_version; exit 0; in the script. You will see now scm/git won't find brew git.

May I suggest to put scm folder inside Homebrew/env because they are in fact env related, except for the fact that they are used all the time by homebrew, while superenv and stdenv are only build related.

MikeMcQuaid · 2016-07-14T08:04:43Z

May I suggest to put scm folder inside Homebrew/env because they are in fact env related, except for the fact that they are used all the time by homebrew, while superenv and stdenv are only build related.

I think we could have a wrappers folder or something but I think env is confusing as it doesn't relate to ENV, superenv or stdenv.

MikeMcQuaid · 2016-07-14T08:10:44Z

You fixed super/git script but broke scm/git in the process.

To check it, you can add echo $brew_version; exit 0; in the script. You will see now scm/git won't find brew git.

Thanks, fixed. I see they assumed they were at the same level before so I've fixed that.

MikeMcQuaid · 2016-07-14T08:53:04Z

@BrewTestBot test this please

MikeMcQuaid · 2016-07-14T14:23:48Z

Will wait and see what @UniqMartin thinks about this (but no rush there, Martin).

UniqMartin · 2016-07-14T18:16:49Z

Library/Homebrew/test/lib/config.rb

@@ -12,7 +12,9 @@

 # Paths pointing into the Homebrew code base that persist across test runs
 HOMEBREW_LIBRARY_PATH  = Pathname.new(File.expand_path("../../..", __FILE__))
-HOMEBREW_ENV_PATH      = HOMEBREW_LIBRARY_PATH.parent+"ENV"
+HOMEBREW_ENV_PATH      = \
+  HOMEBREW_LIBRARY_PATH.parent+"Homebrew/env"


Why does this line need to be wrapped, but the line directly underneath doesn't (though it has the exact same line length when unwrapped)? I think if we wanted to conserve some vertical space here, we'd be better off just dropping the unnecessary whitespace between the constant and the assignment operator.

Bad edits. Will fix 👍

UniqMartin · 2016-07-14T18:32:07Z

👍 on the general cleanup and relocation of Library/ENV to Library/Homebrew. Any reason you renamed ENV to env in the process? ENV in Library/Homebrew/extend is still capitalized and I guess both have some (historical) relationship.

I think we could have a wrappers folder or something but I think env is confusing as it doesn't relate to ENV, superenv or stdenv.

👍 on having wrappers. Or maybe even better support? This sounds like it could be a generic enough name to host the SCM wrappers, the superenv wrappers, as well as the pkgconfig files in their respective subdirectories.

(I haven't thoroughly checked the necessary code changes. I'd prefer to wait with checking whether any of the wrapper scripts are broken after we have agreed on a directory layout.)

Will wait and see what @UniqMartin thinks about this (but no rush there, Martin).

Thanks, this is much appreciated—also in the other related PR! ❤️

MikeMcQuaid · 2016-07-14T18:38:12Z

👍 on the general cleanup and relocation of Library/ENV to Library/Homebrew. Any reason you renamed ENV to env in the process? ENV in Library/Homebrew/extend is still capitalized and I guess both have some (historical) relationship.

I think mainly because I don't think they actually relate to ENV as-in the Ruby class but instead the Superenv and Stdenv classes and hopefully longer-term we can stop monkeypatching ENV 😉

👍 on having wrappers. Or maybe even better support? This sounds like it could be a generic enough name to host the SCM wrappers, the superenv wrappers, as well as the pkgconfig files in their respective subdirectories.

I'm 👍 on wrappers and 👎 on support. Other names: {install-,}{shims,scripts,support,wrappers,helpers} (most of these I'd prefer with an install or similar prefix)?

(I haven't thoroughly checked the necessary code changes. I'd prefer to wait with checking whether any of the wrapper scripts are broken after we have agreed on a directory layout.)

Agreed.

MikeMcQuaid · 2016-07-14T18:38:42Z

I'm 👍 on wrappers and 👎 on support. Other names: {install-,}{shims,scripts,support,wrappers,helpers} (most of these I'd prefer with an install or similar prefix)?

Although I guess technically the install applies to pkgconfig and superenv but not to the scm scripts/wrappers/stubs/helpers.

UniqMartin · 2016-07-14T19:00:05Z

I'm not convinced the install- prefix would add any value. (It actually confuses me a bit.) Of those you mentioned, I think shims and wrappers are the better choices, thus I'd go with shims/scm and shims/superenv for all those shims/wrappers.

The pkgconfig files are really specific to macOS, so I guess os/mac/pkgconfig would be a sensible place to move them to.

Move some stuff formerly in `Library/ENV` around: - Move `Library/ENV/$XCODE_VERSION` to `Library/Homebrew/env/super` as they are all superenv wrappers and all symlinks to the same version. We never needed the "separate shims for separate versions" functionality and it just adds confusion. - Move `Library/ENV/pkgconfig` to `Library/Homebrew/env/pkgconfig` to get more things under `Library/Homebrew` - Move `Library/ENV/scm` to `Library/scm` as these wrappers are not actually used by or related to superenv (or stdenv) in any way.

MikeMcQuaid · 2016-07-14T20:02:12Z

@UniqMartin Done!

andyli · 2016-07-15T18:28:05Z

Look like it causes error when migrating old version of Homebrew to v0.9.9:

/usr/local/Library/Homebrew/cmd/update.sh: line 6: /usr/local/Library/ENV/scm/git: No such file or directory

MikeMcQuaid · 2016-07-15T18:34:53Z

Thanks @andyli, will fix.

bilibiliwwhh · 2016-07-20T05:49:50Z

brew install php55-redis:
/usr/local/opt/php55/bin/phpize: line 61: /usr/local/Library/ENV/4.3/sed: No such file or directory
I have checked ,/usr/local/Library/ENV/ has been removed.

MikeMcQuaid · 2016-07-20T23:11:17Z

@bilibiliwwhh Please report that to Homebrew PHP.

Move some stuff formerly in `Library/ENV` around: - Move `Library/ENV/$XCODE_VERSION` to `Library/Homebrew/env/super` as they are all superenv wrappers and all symlinks to the same version. We never needed the "separate shims for separate versions" functionality and it just adds confusion. - Move `Library/ENV/pkgconfig` to `Library/Homebrew/env/pkgconfig` to get more things under `Library/Homebrew` - Move `Library/ENV/scm` to `Library/scm` as these wrappers are not actually used by or related to superenv (or stdenv) in any way.

BrewTestBot added the in progress Maintainers are working on this label Jul 13, 2016

MikeMcQuaid mentioned this pull request Jul 13, 2016

superenv: port to generic OS. #457

Merged

5 tasks

UniqMartin reviewed Jul 14, 2016
View reviewed changes

MikeMcQuaid merged commit a02be9e into Homebrew:master Jul 15, 2016

BrewTestBot removed the in progress Maintainers are working on this label Jul 15, 2016

MikeMcQuaid deleted the move-superenv-shims branch July 15, 2016 18:09

lovell mentioned this pull request Jul 18, 2016

Possible missing libxml2 depends_on clause in vips.rb homebrew recipe libvips/libvips#485

Closed

Homebrew locked and limited conversation to collaborators May 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENV: move to new paths. #507

ENV: move to new paths. #507

MikeMcQuaid commented Jul 13, 2016

xu-cheng commented Jul 13, 2016 •

edited

Loading

MikeMcQuaid commented Jul 14, 2016

xu-cheng commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

UniqMartin Jul 14, 2016

MikeMcQuaid Jul 14, 2016

UniqMartin commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

UniqMartin commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

andyli commented Jul 15, 2016

MikeMcQuaid commented Jul 15, 2016

bilibiliwwhh commented Jul 20, 2016

MikeMcQuaid commented Jul 20, 2016

ENV: move to new paths. #507

ENV: move to new paths. #507

Conversation

MikeMcQuaid commented Jul 13, 2016

xu-cheng commented Jul 13, 2016 • edited Loading

MikeMcQuaid commented Jul 14, 2016

xu-cheng commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

UniqMartin Jul 14, 2016

Choose a reason for hiding this comment

MikeMcQuaid Jul 14, 2016

Choose a reason for hiding this comment

UniqMartin commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

UniqMartin commented Jul 14, 2016

MikeMcQuaid commented Jul 14, 2016

andyli commented Jul 15, 2016

MikeMcQuaid commented Jul 15, 2016

bilibiliwwhh commented Jul 20, 2016

MikeMcQuaid commented Jul 20, 2016

xu-cheng commented Jul 13, 2016 •

edited

Loading