New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make COMPOSER_BIN_DIR env or _composer_bin_dir global available to binaries #10402
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnstevenson do you see any problem with this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, except that
COMPOSER_BIN_DIR
there will include a trailing\
, if that matters.I'm curious why this
%~dp0
stuff is needed, when you know the location anyway - or will it move?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea why it's there anymore :) I saw the trailing slash, but i am assuming this will safely be ignored by most usages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been in since the very beginning https://github.com/composer/composer/pull/140/files#diff-254824db37e271a109993b63f65d2072eaeeaf7b89996c8ccc0f937cb81ed2d0R173-R195 but perhaps it's not needed at all. Perhaps it was needed to create an absolute path to help with cygwin and such crappy envs. Anyway unless we have a good reason to get rid of it I'd probably leave it in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it was just copied from the way Composer-Setup.exe does it. Whatever, it is certainly not a crucial enough issue to be worrying about at the moment, so I'll shut up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, in case of spaces in directory names it should probably be:
"SET COMPOSER_BIN_DIR=\"%~dp0\"\r\n".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You sure? Because the one above BIN_TARGET does not have it, it only uses quotes when using the variable, not setting it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, you're correct. I had assumed that because BIN_TARGET was being escaped, this would also need it. But it would appear that even though
SET
is a command, it uses its full command-line and ignores any whitespace.Note entirely sure why BIN_TARGET is escaped and then has any surrounding quotes removed, but I'm not going to worry about that.