Summary: | New stage-qa test: infoplist | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | Ports & Packages | Reporter: | Yuri Victorovich <yuri> | ||||||||||||||
Component: | Ports Framework | Assignee: | Port Management Team <portmgr> | ||||||||||||||
Status: | Closed Feedback Timeout | ||||||||||||||||
Severity: | Affects Only Me | CC: | bdrewery | ||||||||||||||
Priority: | --- | ||||||||||||||||
Version: | Latest | ||||||||||||||||
Hardware: | Any | ||||||||||||||||
OS: | Any | ||||||||||||||||
Attachments: |
|
The patch has the little orthogonal change: variable PLIST_SUB_REGEX preventing the code copy-paste. Please leave it in. Created attachment 156412 [details]
Updated patch
Created attachment 156415 [details]
Updated patch
Turns out there are legit .png files under info/. So keeping only info/*.info files banned
This check looks wrong, most of the reported ports are false positives. Can you give an example? In the email you sent, I only had a look at 6 ports and all of them were false positives: ./sysutils/usermin/pkg-plist ./sysutils/webmin/pkg-plist ./sysutils/batmon/pkg-plist ./sysutils/timemon/pkg-plist ./sysutils/virtualmin/pkg-plist ./sysutils/logstash/pkg-plist The list in the e-mail is the result of 'find' command, not of this check. If most of them are false positives, this means that it is very easy to correct the small amount of the rightfully-positive ones. Created attachment 156480 [details]
Updated patch
Updated patch as per your suggestion to use @info. Only the message changed.
You should grep in all pkg-plist files in ports tree and replace all info/*.info with @info *.info once and for good. INFO= does more than @info, it deals with split pages too. But for the matter of this problem, replacing info/*.info with @info *.info should be a safe bet, because they are already in pkg-plist. INFO= adds the RUN_DEPENDS on indexinfo too. No, both of them have to add RUN_DEPENDS on indexinfo, and have to add corresponding post-install and post-deinstall scripts. If @info doesn't do that, this is a bug. No, both of them have to add RUN_DEPENDS on indexinfo, and have to add corresponding post-install and post-deinstall scripts. If @info doesn't do that, this is a bug. Dependency on indexinfo is only added by INFO= This is your opinion that this is a bug, not mine. This is absolutely a bug if this is the case. @info should have exactly the same effect as INFO=. And I tell you it's not a bug with my portmgr hat. You need to explain why. pkg-plist is used to describe the content of a package before processing Makefile/Mk is used to fetch/patch/configure/build/stage the port, to handle dependencies and to add automatically stuff to plist for some variables (PORTDOCS, PORTEXAMPLES, INFO, FONTSDIR, LICENSE, USES...) Sorry, but this doesn't explain why @info doing supposedly less than INFO= is the valid thing to do. When there is the info/*.info file added by the package, it has to be processed by indexinfo every time such package is installed or deinstalled. Hence, RUN_DEPENDS on indexinfo is a must. This is the simple single requirement, and I don't think that @info is able to somehow get away from it in a valid way. @info is simply a convenience macro, and it should do the same as INFO=, unless there are some compelling reasons why they should differ. @info is processed by Mk scripts, and it can, and absolutely should add RUN_DEPENDS. It's the other way, INFO is processed by Mk and it puts @info in post-processed plist + the run depend on indexinfo They both should be processed by Mk, combined into the same list. Otherwise, this is a bug, because allowing to have two levels of .info practically doesn't make sense. It doesn't matter who/how processes them now, if they aren't combined into the same set this is a bug. Also, nobody, ever, should put "@info something" in a plist. It is only added by adding INFO=something in the Makefile. Thank you for pointing out the bug in our documentation, it has been fixed in https://svnweb.freebsd.org/changeset/doc/46668 The main point of this bug was the new stage-qa check 'infoplist'. You didn't check it in yet. Created attachment 156506 [details]
Updated patch
Created attachment 156507 [details]
Updated patch
Antoine Brodin - thanks for pointing out that @info macros aren't allowed in pkg-plist. Mathieu Arnold, thanks likewise. |
Created attachment 156411 [details] patch New 'infoplist' test prevents .info files to be in plist. They should be declared only in INFO= They cause leftover errors in poudriere builds when .info files are in plist.