The ebuild goes at lines 142-143: dosym /${BASE}/bin/kcm_adobe_flash_player.so \ /usr/$(get_libdir)/kde4/ Which looks like incorrect usage of dosym according to http://dev.gentoo.org/~ulm/pms/4/pms.html. Second argument must be full path to intended symlink, not just directory. This very thing breaks install when paludis is used instead of emerge. Reproducible: Always Steps to Reproduce: 1.Try to install said package with paludis. Actual Results: Does not install. Expected Results: Installs.
Internally dosym just calls 'ln -snf $1 $2', so directory as second argument is acceptable for 'ln'. See /usr/lib/portage/bin/ebuild-helpers/dosym.
@maxim: the point of PMS isn't about what portage innards happen to do (which varies across versions/years)- it's about what ebuild authors can *expect*. The ebuild's wrong; the second arg needs to be the full path (name included), rather than just the directory target. @Zac: that invocation should be changed to -T; as is, mkdir foon; dosym blah /foon would invalidly succeed going by a quick read...
(In reply to comment #2) > @Zac: that invocation should be changed to -T; as is, mkdir foon; dosym blah > /foon would invalidly succeed going by a quick read... Looking at the single-unix docs, I'm not sure if -T is portable: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/ln.html Anyway, we can easily emulate -T a line or two of shell code.
(In reply to comment #3) > (In reply to comment #2) > > @Zac: that invocation should be changed to -T; as is, mkdir foon; dosym blah > > /foon would invalidly succeed going by a quick read... > > Looking at the single-unix docs, I'm not sure if -T is portable: > > http://pubs.opengroup.org/onlinepubs/9699919799/utilities/ln.html > > Anyway, we can easily emulate -T a line or two of shell code. The '-n' usage in portage also isn't covered PMS wise; I'd be inclined to retroactively update the spec on that one if it's desirable (it's been in portage that way for a while afaik).
(In reply to comment #2) > The ebuild's wrong; the second arg needs to be the full path (name included), > rather than just the directory target. Looks like PMS is not accurate here, as Portage has always accepted a directory as second argument. IMO we should mention that case in the spec.
Just for the record, the precise reason why this fails in Paludis is that when automatically creating the destination directory, Paludis uses $(dirname ${2}) whereas Portage uses ${2/%}. If ${2} ends in a /, then the Portage version is essentially equivalent to ${2} itself, so Portage creates the required directory and then the ln command creates the symlink as expected. However, the Paludis version only creates the parent of the desired directory, and then ln fails because it's sensitive to the trailing / and expects it to name an existing directory. If we do want to allow this in the spec, then the wording will need to cover this carefully as it's fairly subtle.
"If the second parameter is a directory or ends with a slash, then append the last pathname component of the first parameter, before doing any further action."?
The current portage behavior dates back to v2.1_pre8: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=2f4af673720d8230c31fb72e5686fcae6bbd1ec8
(In reply to comment #5) > (In reply to comment #2) > > The ebuild's wrong; the second arg needs to be the full path (name included), > > rather than just the directory target. > > Looks like PMS is not accurate here, as Portage has always accepted a directory > as second argument. IMO we should mention that case in the spec. If PMS behaviour conflicted with portage, that line of thought would apply; what PMS specifies is a controlled subset of portage behaviour. The PMS form works fine w/ portage- portage (and pkgcore up until a recent rev) were loose in what they allowed however, while paludis wasn't. That notion strongly makes me think the ebuilds need fixing rather than the spec. Offhand, there are ~4800 dosym invocations in the tree; a quick scan of it, about ~80 versions that look to rely on the second arg as a directory (only ~30 w/ explicit / to force it). Mind you, those are *rough* stats, but I'm seeing ~2% potential, w/ <1% confirmed... well, makes me think the ebuilds need a fixin instead ;)
(In reply to comment #9) > If PMS behaviour conflicted with portage, that line of thought would apply; > what PMS specifies is a controlled subset of portage behaviour. The PMS form > works fine w/ portage- portage (and pkgcore up until a recent rev) were loose > in what they allowed however, while paludis wasn't. That notion strongly > makes me think the ebuilds need fixing rather than the spec. The problem is that we cannot prevent such usage in the tree, unless Portage will flag an error for the second argument being a directory. And if it is made an error, then breakage will occur for the non-conforming packages.
Non-conforming packages are already broken. All making it error out would do is make that breakage visible so it can be fixed.
They may be broken, but for most users that doesn't have any effect. Making it "visible" means that we break users' stable systems.
Making it visible means breakage gets identified and fixed.
I've added a QA Notice to portage: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=05b3e2b245630505fa5f581fbdf0531d00cb0b93
(In reply to comment #14) > I've added a QA Notice to portage: > > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=05b3e2b245630505fa5f581fbdf0531d00cb0b93 Shouldn't it also warn if ${D}$2 is an existing directory?
(In reply to comment #15) > (In reply to comment #14) > > I've added a QA Notice to portage: > > > > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=05b3e2b245630505fa5f581fbdf0531d00cb0b93 > > Shouldn't it also warn if ${D}$2 is an existing directory? Okay, done: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=3534aa843f22e84f5dd79a8306605d5178ec76c3
it visible"; making it a qa warning is wrong; it's not QA, it's broken, and the list of offending packages include quite a few crufty ones which aren't going to get updated anytime soon I'd expect. In that respect Ulm, I very much disagree about "we must never break the tree"; the tree is *broken* for 2/3 managers for those packages. Admittedly that percentile of users (versus portage) is less, but the line of thought being pursued there is basically "shit on the compliant implementations" which isn't exactly a great reward for doing things right. Recall EAPI0 was passed, and ebuilds were supposed to match it; those are broke ebuilds. It happens, but you really should be looking at the tree from that angle rather than the angle you/zac are pursuing. As for 3534aa843f22e84f5dd79a8306605d5178ec76c3, the rules of dosym replacing an existing sym/file/whatever are fairly vague; this is an area we should tighten PMS up on. Either way, my stance is convert it into an error; portage sits in unstable for a while upon release, during that interval the offending packages must be updated (I'll generate that list later in the week); regardless of portage, they need updating since they're incorrect and causing breakage.
(In reply to comment #17) > it visible"; making it a qa warning is wrong; it's not QA, it's broken, and the > list of offending packages include quite a few crufty ones which aren't going > to get updated anytime soon I'd expect. Pardon the crap at the front of the comment; unix copy/paste strikes again... ;)
i would consider this a useful feature worth adding to the PMS so that things do work in future EAPIs. `dosym` is analogous to `ln`, and `ln` allows dir arguments where the filename is implicit based on the source basename, so keeping things "least surprise" and "just works" is a good thing.
(In reply to comment #17) > In that respect Ulm, I very much disagree about "we must never break the > tree"; the tree is *broken* for 2/3 managers for those packages. Admittedly > that percentile of users (versus portage) is less, but the line of thought > being pursued there is basically "shit on the compliant implementations" > which isn't exactly a great reward for doing things right. Recall EAPI0 was > passed, and ebuilds were supposed to match it; those are broke ebuilds. It > happens, but you really should be looking at the tree from that angle rather > than the angle you/zac are pursuing. First, I agree that ebuilds must be fixed. Second, the point is not about "not breaking the tree", but about imposing such breakage on our users. Therefore, it should stay a QA warning until we think that we have fixed all non-compliant cases (which is trivial once they've been identified). Only _then_ the warning should be changed into an error.
Perhaps the devmanual (http://devmanual.gentoo.org/function-reference/install-functions/index.html) could also be updated to make note of this specific difference between 'ln -s' and 'dosym'?
...what's going on with this? I'm seeing *new* ebuilds showing up with it...
(In reply to comment #22) > ...what's going on with this? I'm seeing *new* ebuilds showing up with it... Well, the QA notice isn't in stable portage yet. It was released in portage-2.1.10.12 and 2.2.0_alpha52.
(In reply to comment #23) > (In reply to comment #22) > > ...what's going on with this? I'm seeing *new* ebuilds showing up with it... > > Well, the QA notice isn't in stable portage yet. It was released in > portage-2.1.10.12 and 2.2.0_alpha52. Being that it's been 3 months, situation hasn't improved, and realistically this isn't going to be sorted in the next 6, well... adhering to the spec, instead of just changing the fucking spec to be accurate is costing us a lot of pain here. As such, pkgcore 0.7.5-r2 will land in the tree shortly supporting the "2nd arg is a directory" form of dosym. Consider that my strong suggestion that the spec be changed to match reality (since cleaning the tree requires a full tinderbox run), and if people actually want this crap, to add it in EAPI5. Either way, seen enough breakage adhering to the spec, so... y'all sort it out.
The QA warning is in place since more than 5 years, so maybe it is time to make this a fatal error?
Created attachment 470084 [details, diff] dosym: Make implicit basename a fatal error.
(In reply to Ulrich Müller from comment #26) $ fgrep -Hr 'QA Notice: dosym target omits basename:' ~/img?/*/var/log/portage/elog/ | cut -f1-2 -d':' | xargs -n1 basename | sort -u app-emacs:slime-2.0_p20101103 media-gfx:nip2-7.26.4 media-gfx:nip2-7.38.1 www-apps:novnc-0.6.1 www-apps:novnc-0.6.2 ;)
(In reply to Toralf Förster from comment #27) > app-emacs:slime-2.0_p20101103 > media-gfx:nip2-7.26.4 > media-gfx:nip2-7.38.1 > www-apps:novnc-0.6.1 > www-apps:novnc-0.6.2 Thank you. All fixed: f41039250c57 www-apps/novnc: [QA] Use complete path as second argument of dosym. 182444c044c9 media-gfx/nip2: [QA] Use complete path as second argument of dosym. 62a4ef94dfcc app-emacs/slime: Use complete path as second argument of dosym.
(In reply to Ulrich Müller from comment #26) > Created attachment 470084 [details, diff] [details, diff] > dosym: Make implicit basename a fatal error. hm, did we never commit this?
(In reply to Sam James from comment #29) > (In reply to Ulrich Müller from comment #26) > > Created attachment 470084 [details, diff] [details, diff] [details, diff] > > dosym: Make implicit basename a fatal error. > > hm, did we never commit this? https://gitweb.gentoo.org/proj/portage.git/commit/?id=c40c39537d584f57e449d3525e92fafbe85517fd
(In reply to Ulrich Müller from comment #30) > (In reply to Sam James from comment #29) > > (In reply to Ulrich Müller from comment #26) > > > Created attachment 470084 [details, diff] [details, diff] [details, diff] [details, diff] > > > dosym: Make implicit basename a fatal error. > > > > hm, did we never commit this? > > https://gitweb.gentoo.org/proj/portage.git/commit/ > ?id=c40c39537d584f57e449d3525e92fafbe85517fd Thanks!
