Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
OTOH maybe I just need to adopt a merge first strategy to avoid embarrassing accidental closures... Anyway, thanks for the nice cleanups. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-590236669___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
Merged #1071 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#event-3064645142___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
Doh, certainly didn't intend to close but just comment and then merge. Been multiple such mistakes from me in the last week or so, wonder if some button order or such on GH changed. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-590226857___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
Reopened #1071. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#event-3064556851___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
Okay so there was more than meets the eye... thankfully caught by the test-suite. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-590226357___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
Closed #1071. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#event-3064556636___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
@mlschroe pushed 1 commit. 4db183db61ca56d035712ca752234aa7c8e8b097 Only look at symlinks in new packages in fpLookupSubdir -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071/files/fe90d1990af3809b9f1174bbf77e458435154328..4db183db61ca56d035712ca752234aa7c8e8b097 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
@mlschroe pushed 1 commit. fe90d1990af3809b9f1174bbf77e458435154328 Only look at symlinks in new packages in fpLookupSubdir -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071/files/841134ef4697842f1fb3ef4bb2f3993deb8f2a5c..fe90d1990af3809b9f1174bbf77e458435154328 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
@mlschroe pushed 1 commit. 841134ef4697842f1fb3ef4bb2f3993deb8f2a5c Only look at symlinks in new packages in fpLookupSubdir -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071/files/954c491165cacf9156d2e4b0f3afaaaef9c3d529..841134ef4697842f1fb3ef4bb2f3993deb8f2a5c ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
Oh yes, we'll need to do the rpmfilesFpLookup() call for TR_REMOVED packages. Fixing... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-588183440___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
This crashes at the "reinstall 1" test with the following (from valgrind): > fprint.c:262:12: runtime error: member access within null pointer of type > 'const struct fingerPrint' ==943715== Invalid read of size 4 ==943715==at 0x4923867: fpHashFunction (fprint.c:262) ==943715==by 0x492777D: rpmFpHashAddEntry (rpmhash.C:168) ==943715==by 0x4929D42: fpCachePopulate (fprint.c:515) ==943715==by 0x49958BF: rpmtsPrepare (transaction.c:1464) ==943715==by 0x49964CE: rpmtsRun (transaction.c:1721) ==943715==by 0x4973989: rpmcliTransaction (rpminstall.c:291) ==943715==by 0x4977A99: rpmInstall (rpminstall.c:632) ==943715==by 0x10C6C9: main (rpm.c:265) ==943715== Address 0xc is not stack'd, malloc'd or (recently) free'd I guess the second loop will also need to be changed to look at only added packages, or something. For an easy reproducer outside the test-suite: > [root@lumikko-w rpm]# ./rpm -U --nodeps --ignorearch --root /srv/test > tests/data/RPMS/hello-2.0-1.i686.rpm [root@lumikko-w rpm]# ./rpm --reinstall --nodeps --ignorearch --nodocs --root /srv/test/ tests/data/RPMS/hello-2.0-1.i686.rpm -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-588163632___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
@mlschroe pushed 1 commit. 954c491165cacf9156d2e4b0f3afaaaef9c3d529 Only look at symlinks in new packages in fpLookupSubdir -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071/files/1732cc787d6f508019edc7c455aa02d9b9ad78e6..954c491165cacf9156d2e4b0f3afaaaef9c3d529 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
mlschroe commented on this pull request.
> pi = rpmtsiInit(ts);
while ((p = rpmtsiNext(pi, 0)) != NULL) {
fingerPrint *fpList;
(void) rpmsqPoll();
+ if (rpmteType(p) == TR_REMOVED)
+ continue; /* we are only interested in new packages */
Oh, I didn't know about that. Force pushed.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1071#discussion_r381214248___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
pmatilai commented on this pull request.
> pi = rpmtsiInit(ts);
while ((p = rpmtsiNext(pi, 0)) != NULL) {
fingerPrint *fpList;
(void) rpmsqPoll();
+ if (rpmteType(p) == TR_REMOVED)
+ continue; /* we are only interested in new packages */
Why not change the loop itself to only look at added packages instead, eg
rpmtsiNext(pi, TR_ADDED) in the above?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1071#pullrequestreview-361004344___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
I've added a commit that makes the code only consider symlinks. Florian, any objections to this? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-588151243___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
@mlschroe pushed 1 commit. 1732cc787d6f508019edc7c455aa02d9b9ad78e6 Only look at symlinks in new packages in fpLookupSubdir -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071/files/1c9343142a728dce571ee0c8ce4f6fae42354588..1732cc787d6f508019edc7c455aa02d9b9ad78e6 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
Good question, I've no idea. @ffesti? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-587491783___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
I didn't plan to push more commits, but now that you ask: Does it really make sense to have the symlinks of already installed packages in the `symlinks` hash? If a symlink is deleted on disk, the code will ignore it anyway (see the "Ignore already removed (by eg %pretrans) links" comment). If it's still on disk, the fingerprint lookup code will have found it and it will not be part of the "subDir" element. So what's the point? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-587448013___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
> I'm trying to make dir -> symlink-to-dir transactions work in the easy cases, > I think this will be fixed with the changes as well Wow. Good luck with that :grin: As for the slashes, I seem to recall looking at reducing them but something preventing that, perhaps some "hidden" dependency to rpmfi API / users expecting those slashes there and placing them back would've been impossible/hard. But it's a long time ago, things may have changed and my memory is really hazy on the details at best. BTW, are you expecting to push more commits to this particular PR or should we just merge it? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-587424452___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
Regarding your b81b4a35240f16fa8b45156b0151fab9e130a8e8 commit: fpLookupSubdir's slash handling is still somewhat broken, it tends to duplicate slashes when creating the link. The fingerprint lookup fortunately calls rpmCleanPath() with gets rid of the extra slashes again. BTW, why do the subDir entries in the fingerprints both have a leading and a trailing `/`? Is that something we should fix? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-586929882___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
Ok, that's what I thought. But it's somewhat brittle, that example from the mail will not work if FOO-DOC is installed before FOO as then /usr/share/FOO-1 will get created as directory and the install of FOO will fail with a RPMERR_EXIST_AS_DIR error. (I'm trying to make dir -> symlink-to-dir transactions work in the easy cases, I think this will be fixed with the changes as well.) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-586927889___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
Yes, that's pretty much it. The finger printing code calculates a unique identifier for each file's location. This is comprised out of the device id and inode number of the parent dir and the filename. If the parent dir is not on disk yet, the closest dir is used + the path down to the parent to be installed. fpLookupSubdir looks for symlinks that may alias the files in this - not yet installed - part of the path. Yes, it assumes that the symlink actually gets installed. But this is not an unreasonable assumption as we do a file conflict checks for this symlinks, too. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-586885629___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
In case you wonder, I remember due to encountering this during the string-pool mass changes, and I do remember having a manual test-case for commit b81b4a35240f16fa8b45156b0151fab9e130a8e8. Too bad I didn't create an automated test back then :disappointed: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-586872904___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
IIRC this is the reproducer case: http://lists.rpm.org/pipermail/rpm-maint/2008-April/002051.html -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-586870800___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
Ok, I have to admit I'm not entirely sure what fpLookupSubdir is trying to fix. It was added by Florian in commits c6ccc90d7fef0f1b65e4bf5b77d5b800d4b53ffd and af3464a053ecb0b56cc5af494ea22955fb350757, unfortunately without a reference to some bug. It seems to be about having a symlink in one to-be-installed package and then installing over that symlink in another to-be-installed package, but that use case does not work reliable anyway because the symlink package must be installed first for it to work. Florian, do you remember more details what this was about? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071#issuecomment-586331219___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] More fpLookupSubdir cleanups (#1071)
@mlschroe pushed 1 commit. 1c9343142a728dce571ee0c8ce4f6fae42354588 Reduce the number of calls to fpLookupSubdir() -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1071/files/0fdc9a865711454ceb8312bc50a6c22d1b2ceb15..1c9343142a728dce571ee0c8ce4f6fae42354588 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
