Skip to content
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 1 commit into from Dec 28, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 9 additions & 4 deletions src/Composer/Installer/BinaryInstaller.php
Expand Up @@ -231,12 +231,14 @@ protected function generateWindowsProxyCode($bin, $link)
return "@ECHO OFF\r\n".
"setlocal DISABLEDELAYEDEXPANSION\r\n".
"SET BIN_TARGET=%~dp0/".trim(ProcessExecutor::escape(basename($link, '.bat')), '"\'')."\r\n".
"SET COMPOSER_BIN_DIR=%~dp0\r\n".
"{$caller} \"%BIN_TARGET%\" %*\r\n";
}

return "@ECHO OFF\r\n".
"setlocal DISABLEDELAYEDEXPANSION\r\n".
"SET BIN_TARGET=%~dp0/".trim(ProcessExecutor::escape($binPath), '"\'')."\r\n".
"SET COMPOSER_BIN_DIR=%~dp0\r\n".
Copy link
Member Author

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?

Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member Author

@Seldaek Seldaek Dec 28, 2021

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.

Copy link
Member

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!

Copy link
Member

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".

Copy link
Member Author

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.

Copy link
Member

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.

"{$caller} \"%BIN_TARGET%\" %*\r\n";
}

Expand All @@ -260,14 +262,15 @@ protected function generateUnixyProxyCode($bin, $link)
// carry over the existing shebang if present, otherwise add our own
$proxyCode = empty($match[1]) ? '#!/usr/bin/env php' : trim($match[1]);
$binPathExported = $this->filesystem->findShortestPathCode($link, $bin, false, true);
$autoloadPathCode = $streamProxyCode = $streamHint = '';
$streamProxyCode = $streamHint = '';
$globalsCode = '$GLOBALS[\'_composer_bin_dir\'] = __DIR__;'."\n";
// Don't expose autoload path when vendor dir was not set in custom installers
if ($this->vendorDir) {
$autoloadPathCode = '$GLOBALS[\'_composer_autoload_path\'] = ' . $this->filesystem->findShortestPathCode($link, $this->vendorDir . '/autoload.php', false, true).";\n";
$globalsCode .= '$GLOBALS[\'_composer_autoload_path\'] = ' . $this->filesystem->findShortestPathCode($link, $this->vendorDir . '/autoload.php', false, true).";\n";
}
// Add workaround for PHPUnit process isolation on PHPUnit 6+
if ($this->filesystem->normalizePath($bin) === $this->filesystem->normalizePath($this->vendorDir.'/phpunit/phpunit/phpunit')) {
$autoloadPathCode .= '$GLOBALS[\'__PHPUNIT_ISOLATION_EXCLUDE_LIST\'] = $GLOBALS[\'__PHPUNIT_ISOLATION_BLACKLIST\'] = array(realpath('.$binPathExported.'));'."\n";
$globalsCode .= '$GLOBALS[\'__PHPUNIT_ISOLATION_EXCLUDE_LIST\'] = $GLOBALS[\'__PHPUNIT_ISOLATION_BLACKLIST\'] = array(realpath('.$binPathExported.'));'."\n";
}
if (trim($match[0]) !== '<?php') {
$streamHint = ' using a stream wrapper to prevent the shebang from being output on PHP<8'."\n *";
Expand Down Expand Up @@ -368,7 +371,7 @@ public function stream_set_option(\$option, \$arg1, \$arg2)

namespace Composer;

$autoloadPathCode
$globalsCode
$streamProxyCode
include $binPathExported;

Expand All @@ -389,6 +392,8 @@ public function stream_set_option(\$option, \$arg1, \$arg2)
esac
fi

export COMPOSER_BIN_DIR=\$(cd "\${0%[/\\\\]*}" > /dev/null; pwd)

"\${dir}/$binFile" "\$@"

PROXY;
Expand Down