Bug 27707

On Mon, May 28, 2001 at 03:40:39PM +0200, tobez@tobez.org wrote:
> 
> >Number:         27707
> >Category:       ports
> >Synopsis:       Bogus make errors while executing pkg_version -v
> >Originator:     Anton Berezin
> >Release:        FreeBSD 5.0-CURRENT i386
> >Organization:
> catpipe Systems ApS
> >Environment:
> 
> Any FreeBSD system with ORIGIN-aware ports collection.
> 
> >Description:
> 
> 	# sh /etc/periodic/weekly/400.status-pkg 
> 
> 	Check for out of date packages:
> 	make: no target to make.
> 	make: no target to make.
> 	  AbiWord-0.7.13 needs updating (port has 0.7.13_1)
>     ...
> 
> These bogus make errors appear because pkg_version does not check for
> the existence of the Makefile in the port skeleton directory.  Under
> certain circumstances it is possible to have stale ports directories
> with only README.html file in them.
>
> >How-To-Repeat:
> 
> If you ports collection was installed by sysinstall (i.e., it has
> README.html files), and the skeletons of some of the ports you have
> installed were subsequently nuked by cvsup, you will see these make
> error messages when you execute pkg_version.
> 
> >Fix:
> 
> Index: pkg_version.pl
> ===================================================================
> RCS file: /home/ncvs/src/usr.sbin/pkg_install/version/pkg_version.pl,v
> retrieving revision 1.20
> diff -u -r1.20 pkg_version.pl
> --- pkg_version.pl	2001/05/15 18:37:23	1.20
> +++ pkg_version.pl	2001/05/28 13:28:38
> @@ -374,6 +374,7 @@
>  	# The chdir needs to be successful or our make -V invocation
>  	# will fail.
>  	chdir "$PortsDirectory/$origin" or next;
> +	next unless -r "Makefile";
>  
>  	open PKGNAME, "$GetPkgNameCommand|";
>  	$pkgname = <PKGNAME>;

Most of these cases involve ports which were removed altogether or
repo-copied into another category.

I personally think that both these cases merit the user's attention -
in the case of a removed port, the CVS log messages would show
a possible reason - obsoleted by this-and-this, or removed for lack
of attention and a bad security track, or something similar;
in the case of a repo-copied port, it is highly in the admin's interest
to either just change the origin line in /var/db/pkg/name/+CONTENTS,
or rebuild the port, to make sure that subsequent pkg_version runs
report correct version information.

So how about this instead:

Index: src/usr.sbin/pkg_install/version/pkg_version.pl
===================================================================
RCS file: /home/ncvs/src/usr.sbin/pkg_install/version/pkg_version.pl,v
retrieving revision 1.4.2.11
diff -u -r1.4.2.11 pkg_version.pl
--- src/usr.sbin/pkg_install/version/pkg_version.pl	2001/05/18 14:40:44	1.4.2.11
+++ src/usr.sbin/pkg_install/version/pkg_version.pl	2001/05/28 17:19:13
@@ -373,7 +373,10 @@
 	# Try to get the version out of the makefile.
 	# The chdir needs to be successful or our make -V invocation
 	# will fail.
-	chdir "$PortsDirectory/$origin" or next;
+	if (!(chdir("$PortsDirectory/$origin") && -r "Makefile")) {
+		warn("orphaned: $packageString: $PortsDirectory/$origin\n");
+		next;
+	}
 
 	open PKGNAME, "$GetPkgNameCommand|";
 	$pkgname = <PKGNAME>;

..making pkg_version actually whine about the change?

G'luck,
Peter

-- 
I am not the subject of this sentence.

On Mon, May 28, 2001 at 08:25:15PM +0300, Peter Pentchev wrote:

> Most of these cases involve ports which were removed altogether or
> repo-copied into another category.
> 
> I personally think that both these cases merit the user's attention -
> in the case of a removed port, the CVS log messages would show
> a possible reason - obsoleted by this-and-this, or removed for lack
> of attention and a bad security track, or something similar;
> in the case of a repo-copied port, it is highly in the admin's interest
> to either just change the origin line in /var/db/pkg/name/+CONTENTS,
> or rebuild the port, to make sure that subsequent pkg_version runs
> report correct version information.

I agree, but how's that different from the current `unknown in index'
report for such packages?

\Anton.
-- 
May the tuna salad be with you.

On Mon, May 28, 2001 at 07:40:26PM +0200, Anton Berezin wrote:
> On Mon, May 28, 2001 at 08:25:15PM +0300, Peter Pentchev wrote:
> 
> > Most of these cases involve ports which were removed altogether or
> > repo-copied into another category.
> > 
> > I personally think that both these cases merit the user's attention -
> > in the case of a removed port, the CVS log messages would show
> > a possible reason - obsoleted by this-and-this, or removed for lack
> > of attention and a bad security track, or something similar;
> > in the case of a repo-copied port, it is highly in the admin's interest
> > to either just change the origin line in /var/db/pkg/name/+CONTENTS,
> > or rebuild the port, to make sure that subsequent pkg_version runs
> > report correct version information.
> 
> I agree, but how's that different from the current `unknown in index'
> report for such packages?

It is different for:
  1. repo-copied packages with the same version, which are *still*
     in the index with the same name, and
  2. packages deleted since the last index rebuild (which can take
     a while - after all, people do have things to do other than
     rebuild port indices every now and then :)

Both of these will be 'fixed' as soon as the index is rebuilt, but
still, I think a fast notification that something is wrong would
be preferable.

G'luck,
Peter

-- 
Nostalgia ain't what it used to be.

On Mon, May 28, 2001 at 08:55:05PM +0300, Peter Pentchev wrote:
> On Mon, May 28, 2001 at 07:40:26PM +0200, Anton Berezin wrote:
> > On Mon, May 28, 2001 at 08:25:15PM +0300, Peter Pentchev wrote:
> > 
> > > Most of these cases involve ports which were removed altogether or
> > > repo-copied into another category.
> > 
> > I agree, but how's that different from the current `unknown in index'
> > report for such packages?
> 
> It is different for:
>   1. repo-copied packages with the same version, which are *still*
>      in the index with the same name, and
>   2. packages deleted since the last index rebuild (which can take
>      a while - after all, people do have things to do other than
>      rebuild port indices every now and then :)
> 
> Both of these will be 'fixed' as soon as the index is rebuilt, but
> still, I think a fast notification that something is wrong would
> be preferable.

Hmm, okay.  In this case the better fix would be to actually use the
same reporting mechanism pgk_version(1) already uses, instead of getting
away with warn.

What do you think?

Index: pkg_version.pl
===================================================================
RCS file: /home/ncvs/src/usr.sbin/pkg_install/version/pkg_version.pl,v
retrieving revision 1.20
diff -u -r1.20 pkg_version.pl
--- pkg_version.pl	2001/05/15 18:37:23	1.20
+++ pkg_version.pl	2001/05/28 18:24:04
@@ -373,7 +373,10 @@
 	# Try to get the version out of the makefile.
 	# The chdir needs to be successful or our make -V invocation
 	# will fail.
-	chdir "$PortsDirectory/$origin" or next;
+	unless (chdir "$PortsDirectory/$origin" and -r "Makefile") {
+	    $currentPackages{$packageString}->{orphaned} = 1;
+	    next;
+	}
 
 	open PKGNAME, "$GetPkgNameCommand|";
 	$pkgname = <PKGNAME>;
@@ -448,8 +451,15 @@
     $packageName = $currentPackages{$packageString}{'name'};
 
     $currentVersion = $currentPackages{$packageString}{'fullversion'};
+
+    if ($currentPackages{$packageString}->{orphaned}) {
+
+	next if $ShowCommandsFlag;
+	$versionCode = "?";
+	$Comment = "orphaned";
+
+    } elsif (defined $currentPackages{$packageString}{'portversion'}) {
 
-    if (defined $currentPackages{$packageString}{'portversion'}) {
 	$portVersion = $currentPackages{$packageString}{'portversion'};
 
 	$portPath = "$PortsDirectory/$currentPackages{$packageString}{'origin'}";


%Anton.
-- 
May the tuna salad be with you.

OK, let's get this into the GNATS audit-trail; comments inline.

On Mon, May 28, 2001 at 09:26:42PM +0200, Anton Berezin wrote:
> On Mon, May 28, 2001 at 10:08:02PM +0300, Peter Pentchev wrote:
> 
> > OK, how about this: reporting the no-longer-existent directory, too?
> 
> Almost.  :-)  "$PortsDirectory/$origin" is too long, and there is no
> need for concatenation in the report line - the string interpolation
> will do.

Oh ok, I wondered if it could handle -> and such :)

> Now, would you commit it, or shall we wait for Bruce's opinion?  ;-)
>
> By the way, it will be necessary to add an extra sed line to
> 400.status-pkg (see a smallish patch below the main patch).
> 
> =Anton.
> -- 
> May the tuna salad be with you.

I don't think I have the karma to mess around the src/ tree :)
I think we should wait for bmah or mharo to give their opinion.

Other than that, your patch seems just fine to me :)

G'luck,
Peter

-- 
I've heard that this sentence is a rumor.

> Index: pkg_version.pl
> ===================================================================
> RCS file: /home/ncvs/src/usr.sbin/pkg_install/version/pkg_version.pl,v
> retrieving revision 1.20
> diff -u -r1.20 pkg_version.pl
> --- pkg_version.pl	2001/05/15 18:37:23	1.20
> +++ pkg_version.pl	2001/05/28 19:16:01
> @@ -373,7 +373,10 @@
>  	# Try to get the version out of the makefile.
>  	# The chdir needs to be successful or our make -V invocation
>  	# will fail.
> -	chdir "$PortsDirectory/$origin" or next;
> +	unless (chdir "$PortsDirectory/$origin" and -r "Makefile") {
> +	    $currentPackages{$packageString}->{orphaned} = $origin;
> +	    next;
> +	}
>  
>  	open PKGNAME, "$GetPkgNameCommand|";
>  	$pkgname = <PKGNAME>;
> @@ -448,8 +451,15 @@
>      $packageName = $currentPackages{$packageString}{'name'};
>  
>      $currentVersion = $currentPackages{$packageString}{'fullversion'};
> +
> +    if ($currentPackages{$packageString}->{orphaned}) {
> +
> +	next if $ShowCommandsFlag;
> +	$versionCode = "?";
> +	$Comment = "orphaned: $currentPackages{$packageString}->{orphaned}";
> +
> +    } elsif (defined $currentPackages{$packageString}{'portversion'}) {
>  
> -    if (defined $currentPackages{$packageString}{'portversion'}) {
>  	$portVersion = $currentPackages{$packageString}{'portversion'};
>  
>  	$portPath = "$PortsDirectory/$currentPackages{$packageString}{'origin'}";
> 
> 
> 
> 
> 
> 
> Index: 400.status-pkg
> ===================================================================
> RCS file: /home/ncvs/src/etc/periodic/weekly/400.status-pkg,v
> retrieving revision 1.6
> diff -u -r1.6 400.status-pkg
> --- 400.status-pkg	2001/04/28 16:15:50	1.6
> +++ 400.status-pkg	2001/05/28 19:25:05
> @@ -21,7 +21,8 @@
>  		-e '/^[^ ]*-\([^ ]*\)  *\* *multiple versions.*[ ,]\1[,)].*/d' \
>  		-e 's/^\([^ ]*\)  *\* *multiple versions.*\((.*\)/  \1 needs updating \2/p' \
>  		-e 's/^\(bsdpan-[^ ]*\)  *? *unknown in index/  \1 may be outdated - check CPAN version manually/p' \
> -		-e 's/^\([^ ]*-[^ ]*\)  *? *unknown in index/  \1 is obsolete/p' |
> +		-e 's/^\([^ ]*-[^ ]*\)  *? *unknown in index/  \1 is obsolete/p' \
> +		-e 's/^\([^ ]*-[^ ]*\)  *? *\(orphaned:.*\)$/  \1 was \2/p' |
>  	    tee /dev/stderr |
>  	    wc -l)
>  	[ $rc -gt 1 ] && rc=1;;

Comment 6 Bruce A. Mah 2001-05-31 17:54:00 UTC

If memory serves me right, Anton Berezin wrote:
> On Mon, May 28, 2001 at 08:55:05PM +0300, Peter Pentchev wrote:
> > On Mon, May 28, 2001 at 07:40:26PM +0200, Anton Berezin wrote:
> > > On Mon, May 28, 2001 at 08:25:15PM +0300, Peter Pentchev wrote:
> > > 
> > > > Most of these cases involve ports which were removed altogether or
> > > > repo-copied into another category.
> > > 
> > > I agree, but how's that different from the current `unknown in index'
> > > report for such packages?
> > 
> > It is different for:
> >   1. repo-copied packages with the same version, which are *still*
> >      in the index with the same name, and
> >   2. packages deleted since the last index rebuild (which can take
> >      a while - after all, people do have things to do other than
> >      rebuild port indices every now and then :)
> > 
> > Both of these will be 'fixed' as soon as the index is rebuilt, but
> > still, I think a fast notification that something is wrong would
> > be preferable.
> 
> Hmm, okay.  In this case the better fix would be to actually use the
> same reporting mechanism pgk_version(1) already uses, instead of getting
> away with warn.
> 
> What do you think?

[snip]

Hi guys--

Sorry I haven't been participating in this email thread...I'm dealing 
with some RELNOTESng stuff as well as lots of work-related things.

I'll take a look at this as soon as I get a chance, idea sounds pretty 
good though.

Thanks,

Bruce.

Comment 7 Bruce A. Mah 2001-05-31 17:54:13 UTC

Responsible Changed
From-To: freebsd-ports->bmah

Make sure I deal with this at some point.

Comment 8 Anton Berezin 2001-06-11 22:32:52 UTC

State Changed
From-To: open->closed

Committed the most recent patch.