Skip to content

Conversation

@Ponce
Copy link
Contributor

@Ponce Ponce commented May 19, 2015

slackware current, php-5.6.8: spotted wrong link /usr/bin/phar.
seems an oversight present also in master.

Or we got a symlink pointing to the build files
@smalyshev
Copy link
Contributor

I'm not sure this is correct - if phar.phar is installed in $(INSTALL_ROOT)$(bindir) why the link would be in $(bindir)?

@Ponce
Copy link
Contributor Author

Ponce commented May 21, 2015

because $(INSTALL_ROOT) is used to install file in a temporary location, for example when packaging, while $(bindir) is the location in the filesystem when the package is installed: in Slackware case, the vanilla result installing the package built without the patch is

$ ls -la /usr/bin/phar
lrwxrwxrwx 1 root root 35 mag 8 20:50 /usr/bin/phar -> /tmp/package-php//usr/bin/phar.phar

but there's nothing in /tmp/package-php//usr/bin/phar.phar (the link is broken) and the invocation of phar from the command line fails

$ ls -la /tmp/package-php//usr/bin/phar.phar
/bin/ls: cannot access /tmp/package-php//usr/bin/phar.phar: No such file or directory
$ ls -la /usr/bin/phar.phar
-rwxr-xr-x 1 root root 14823 apr 24 20:32 /usr/bin/phar.phar

that's why when the link is created the destination it should point to ought to be the final one in the filesystem.

@Ponce
Copy link
Contributor Author

Ponce commented May 21, 2015

driven by the curiosity I've checked how others do: fedora people are forced too fixing this manually when they assemble the package (source http://pkgs.fedoraproject.org/cgit/php.git/plain/php.spec)

(cd $RPM_BUILD_ROOT%{_bindir}; ln -sfn phar.phar phar)

and so opensuse (source https://build.opensuse.org/package/view_file/devel:languages:php/php5/php5.spec)

rm %{buildroot}%{_bindir}/phar
%{__ln_s} -f %{_bindir}/phar.phar %{buildroot}%{_bindir}/phar

@remicollet
Copy link
Member

Perhaps even a simple (relative link are relative to directory of the link)

  $(LN_S) -f phar.phar $(INSTALL_ROOT)$(bindir)/phar

For downstream packaging link to full path is not correct (can cause some issue in chrooted env, another reason why it is recreated in the fedora .spec file).

@remicollet
Copy link
Member

@Ponce is there a bug report about this ? (required for NEWS)

@Ponce
Copy link
Contributor Author

Ponce commented May 21, 2015

I think the reason why is recreated in the fedora .spec file it's because it's broken, as I explained above.
I don't like fedora way because having a link phar in /usr/bin without the full path (/usr/bin/phar.phar) can create problems if someone has another binary called phar.phar in another location that comes first in PATH (~/bin/phar.phar or whatever): this other is called first by the symlink in this case.

regarding the bug report I gotta say I haven't checked...

@remicollet
Copy link
Member

I think this have to be fixed in all branches (5.5+)

@remicollet
Copy link
Member

I don't like fedora way because having a link phar in /usr/bin without the full path (/usr/bin/phar.phar) can create problems if someone has another binary called phar.phar in another location that comes first in PATH

No.
As said previously, relative link are relative to link directory. If /usr/bin/phar => phar.phar, "phar" will use /usr/bin/phar.phar and no other scripts.

Example, having ~/bin/phar.phar (echo "Damned") and /usr/bin/phar.phar

$ ll /usr/bin/phar
lrwxrwxrwx. 1 root root 9 15 mai   10:07 /usr/bin/phar -> phar.phar
$ phar.phar 
damned
$ phar
No command given, check /usr/bin/phar help

@Ponce
Copy link
Contributor Author

Ponce commented May 21, 2015

ah, ok, sorry for the noise. :)

do you want me to do another pull request?

@remicollet
Copy link
Member

@Ponce yes please, and a bug report

@Ponce
Copy link
Contributor Author

Ponce commented May 21, 2015

ok, hope I have done everything correctly

#1294
https://bugs.php.net/bug.php?id=69680

@Ponce Ponce closed this May 21, 2015
@remicollet
Copy link
Member

ok, hope I have done everything correctly

Yes, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants