Bug 220950 - Mk/Scripts/check-stagedir.sh false negatives (with %%OPTIONS%%)
Summary: Mk/Scripts/check-stagedir.sh false negatives (with %%OPTIONS%%)
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Port Management Team
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-23 19:21 UTC by Gerald Pfeifer
Modified: 2017-10-12 16:15 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gerald Pfeifer freebsd_committer 2017-07-23 19:21:20 UTC
It appears that Mk/Scripts/check-stagedir.sh completely misses files that appear
 - in the staging directory, and
 - are listed in pkg-plist with an %%OPTION%% prefix when OPTION is off.

Those files should be included in packages (or when the port is installed), but
are not.

And check-stagedir.sh should diagnose that those files are in stagedir, but not
included in pkg-plist.

This is quite a problem!


How to reproduce:

 1. chdir $PORTSDIR/emulators/wine-devel
 2. svn up -r 445571  (the version before I'll fix pkg-plist in a minute)
 3. Run the following script (after adjusting STAGEDIR and PREFIX)

#!/bin/sh

STAGEDIR=$WRKDIRPREFIX/wine-devel-work/stage
PREFIX=$WRKDIRPREFIX/wine-devel-prefix

grep %%STAGING%% pkg-plist | while read f; do
  f=`echo $f | sed -e 's/%%STAGING%%//'`
  if [ -f "$STAGEDIR/$PREFIX/$f" ]; then
    printf "%s no longer contigent on %%STAGING%%?\n" "$f"
  fi
done
Comment 1 commit-hook freebsd_committer 2017-07-23 20:00:13 UTC
A commit references this bug:

Author: gerald
Date: Sun Jul 23 19:59:17 UTC 2017
New revision: 446505
URL: https://svnweb.freebsd.org/changeset/ports/446505

Log:
  Move some twenty files that originally were part of the Wine Staging
  patchset when Wine 2.0 branched and have been integrated into the Wine
  development tree since then from being only included in packages / installed
  ports when option STAGING is active to the general case (i.e., omit
  the leading %%STAGING%% marker from their entries in pkg-plist).

  This points to an unfortunate deficiency in Mk/Scripts/check-stagedir.sh
  which we are relying on. [1]

  PR:		220950 [1]

Changes:
  head/emulators/wine-devel/Makefile
  head/emulators/wine-devel/pkg-plist
Comment 2 Mathieu Arnold freebsd_committer 2017-07-25 22:31:36 UTC
(In reply to Gerald Pfeifer from comment #0)
> It appears that Mk/Scripts/check-stagedir.sh completely misses files that
> appear
>  - in the staging directory, and
>  - are listed in pkg-plist with an %%OPTION%% prefix when OPTION is off.
> 
> Those files should be included in packages (or when the port is installed),
> but
> are not.

No they should not.

The files should only be included if opt is enabled.

There are many, many, ports that stage all the files but only package them if some options are enabled.

> And check-stagedir.sh should diagnose that those files are in stagedir, but
> not included in pkg-plist.

Well, it might be an idea to say something, like "this file is installed but not packaged" but it must not be either a warning or an error.

> This is quite a problem!

This is a problem in *1* port, but "fixing" it in the framework would break many, many ports.

> 
> How to reproduce:
> 
>  1. chdir $PORTSDIR/emulators/wine-devel
>  2. svn up -r 445571  (the version before I'll fix pkg-plist in a minute)
>  3. Run the following script (after adjusting STAGEDIR and PREFIX)
> 
> #!/bin/sh
> 
> STAGEDIR=$WRKDIRPREFIX/wine-devel-work/stage
> PREFIX=$WRKDIRPREFIX/wine-devel-prefix
> 
> grep %%STAGING%% pkg-plist | while read f; do
>   f=`echo $f | sed -e 's/%%STAGING%%//'`
>   if [ -f "$STAGEDIR/$PREFIX/$f" ]; then
>     printf "%s no longer contigent on %%STAGING%%?\n" "$f"
>   fi
> done
Comment 3 Gerald Pfeifer freebsd_committer 2017-07-26 07:02:37 UTC
(In reply to Mathieu Arnold from comment #2)
> The files should only be included if opt is enabled.

No. ;-)  What happens here is that these files only were generated due to
some patches in the Wine Staging patchset which is enabled via the STAGING
option.

Over time those patches tend to move from Wine Staging into mainline Wine,
at which point they are built by default (without the Staging patchset and
hence option STAGING) and need to be included unconditionally.

> There are many, many, ports that stage all the files but only package
> them if some options are enabled.

Hmm, in that case it becomes a little trickier indeed.  Good point.

> Well, it might be an idea to say something, like "this file is installed
> but not packaged" but it must not be either a warning or an error.

That would be lovely.  Note, this caused real user issues in case of Wine,
so is worth addressing (and we don't know which other ports may be affected).
Comment 4 commit-hook freebsd_committer 2017-10-04 20:24:19 UTC
A commit references this bug:

Author: gerald
Date: Wed Oct  4 20:24:00 UTC 2017
New revision: 451245
URL: https://svnweb.freebsd.org/changeset/ports/451245

Log:
  Add a new Makefile target check-wine-devel-vs-wine-staging that works
  around a deficiency of Mk/Scripts/check-stagedir.sh that does not spot
  entries in pkg-plist that move from being enabled only with the STAGING
  option (%%STAGING%%foo/bar/file) to being there by default (foo/bar/file).

  This has caused actual issues for users, so we need to regularly check
  for such cases, which a simple `check-wine-devel-vs-wine-staging` will
  now do, provided the staging directory is accessible.

  PR:		220950

Changes:
  head/emulators/wine-devel/Makefile
Comment 5 commit-hook freebsd_committer 2017-10-12 16:15:41 UTC
A commit references this bug:

Author: gerald
Date: Thu Oct 12 16:15:11 UTC 2017
New revision: 451904
URL: https://svnweb.freebsd.org/changeset/ports/451904

Log:
  include/wine/windows/d3d11_2, include/wine/windows/d3d11_3 and
  include/wine/windows/d3d11_4 actually are not contingent on the
  STAGING option, so always package them.

  PR:		220950

Changes:
  head/emulators/wine-devel/Makefile
  head/emulators/wine-devel/pkg-plist