Bug 199978 - New stage-qa test: infoplist
Summary: New stage-qa test: infoplist
Status: Closed Feedback Timeout
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Port Management Team
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-05 23:40 UTC by Yuri Victorovich
Modified: 2016-04-13 11:32 UTC (History)
1 user (show)

See Also:


Attachments
patch (2.65 KB, patch)
2015-05-05 23:40 UTC, Yuri Victorovich
no flags Details | Diff
Updated patch (2.68 KB, patch)
2015-05-06 00:07 UTC, Yuri Victorovich
no flags Details | Diff
Updated patch (2.68 KB, patch)
2015-05-06 02:21 UTC, Yuri Victorovich
no flags Details | Diff
Updated patch (2.68 KB, patch)
2015-05-07 21:51 UTC, Yuri Victorovich
no flags Details | Diff
Updated patch (2.85 KB, patch)
2015-05-08 11:14 UTC, Yuri Victorovich
no flags Details | Diff
Updated patch (2.84 KB, patch)
2015-05-08 11:16 UTC, Yuri Victorovich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuri Victorovich freebsd_committer freebsd_triage 2015-05-05 23:40:41 UTC
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.
Comment 1 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-05 23:44:48 UTC
The patch has the little orthogonal change: variable PLIST_SUB_REGEX preventing the code copy-paste. Please leave it in.
Comment 2 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-06 00:07:27 UTC
Created attachment 156412 [details]
Updated patch
Comment 3 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-06 02:21:38 UTC
Created attachment 156415 [details]
Updated patch

Turns out there are legit .png files under info/. So keeping only info/*.info files banned
Comment 4 Antoine Brodin freebsd_committer freebsd_triage 2015-05-06 09:10:55 UTC
This check looks wrong,  most of the reported ports are false positives.
Comment 5 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-06 09:34:08 UTC
Can you give an example?
Comment 6 Antoine Brodin freebsd_committer freebsd_triage 2015-05-06 09:38:36 UTC
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
Comment 7 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-06 09:41:40 UTC
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.
Comment 8 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-07 21:51:56 UTC
Created attachment 156480 [details]
Updated patch

Updated patch as per your suggestion to use @info. Only the message changed.
Comment 9 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-07 22:42:27 UTC
You should grep in all pkg-plist files in ports tree and replace all info/*.info with @info *.info once and for good.
Comment 10 Antoine Brodin freebsd_committer freebsd_triage 2015-05-07 22:44:10 UTC
INFO= does more than @info,  it deals with split pages too.
Comment 11 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-07 23:39:33 UTC
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.
Comment 12 Antoine Brodin freebsd_committer freebsd_triage 2015-05-08 06:14:21 UTC
INFO= adds the RUN_DEPENDS on indexinfo too.
Comment 13 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-08 06:21:53 UTC
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.
Comment 14 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-08 06:21:54 UTC
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.
Comment 15 Antoine Brodin freebsd_committer freebsd_triage 2015-05-08 06:23:55 UTC
Dependency on indexinfo is only added by INFO=
This is your opinion that this is a bug, not mine.
Comment 16 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-08 07:00:25 UTC
This is absolutely a bug if this is the case. @info should have exactly the same effect as INFO=.
Comment 17 Antoine Brodin freebsd_committer freebsd_triage 2015-05-08 07:04:26 UTC
And I tell you it's not a bug with my portmgr hat.
Comment 18 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-08 07:31:36 UTC
You need to explain why.
Comment 19 Antoine Brodin freebsd_committer freebsd_triage 2015-05-08 09:05:51 UTC
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...)
Comment 20 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-08 09:25:44 UTC
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.
Comment 21 Antoine Brodin freebsd_committer freebsd_triage 2015-05-08 09:29:29 UTC
It's the other way,  INFO is processed by Mk and it puts @info in post-processed plist + the run depend on indexinfo
Comment 22 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-08 09:35:16 UTC
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.
Comment 23 Mathieu Arnold freebsd_committer freebsd_triage 2015-05-08 09:38:39 UTC
Also, nobody, ever, should put "@info something" in a plist.

It is only added by adding INFO=something in the Makefile.
Comment 24 Mathieu Arnold freebsd_committer freebsd_triage 2015-05-08 10:51:21 UTC
Thank you for pointing out the bug in our documentation, it has been fixed in https://svnweb.freebsd.org/changeset/doc/46668
Comment 25 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-08 11:05:54 UTC
The main point of this bug was the new stage-qa check 'infoplist'.
You didn't check it in yet.
Comment 26 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-08 11:14:37 UTC
Created attachment 156506 [details]
Updated patch
Comment 27 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-08 11:16:10 UTC
Created attachment 156507 [details]
Updated patch
Comment 28 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-08 11:17:16 UTC
Antoine Brodin - thanks for pointing out that @info macros aren't allowed in pkg-plist.
Comment 29 Yuri Victorovich freebsd_committer freebsd_triage 2015-05-08 11:18:31 UTC
Mathieu Arnold, thanks likewise.