tech-pkg archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Making DESTDIR support mandatory



Thomas Klausner <wiz%NetBSD.org@localhost> writes:

> Since the branch is cut, perhaps we can progress on the destdir issue.
>
> Btw, thanks for the people who converted another few packages to
> destdir support!
>
> I suggest the attached patch -- this makes packages not build if they
> are not using destdir support, as incentive for people interested in
> them to fix them.
>
> We can then decide before the next branch if we want to revert the
> patch, just leave it in (my suggestion), or go further and actually
> remove the BROKEN packages.

Right now, using DESTDIR is not the default.   I certainly agree that
this should change.

Your patch seems to be doing things in wrong order.  Specifically, I
don't understand why it is reasonable to mark packages broken because of
lack of DESTDIR support when the default is not to use DESTDIR support.
I just checked - at least on the 2011Q4 branch, 'make replace' on a
package uses DESTDIR with PKG_DEVELOPER=yes, and doesn't otherwise.  (I
have PKG_DEVELOPER=yes on almost all machines.)

Steps that I think should happen are:

  1) Change default to use DESTDIR, with or without PKG_DEVELOPER.
  Start having the norm for bulk builds to use this (perhaps they do,
  but I'd expect default behavior for public bulk builds).  Have an
  option to set it to the old way.  This is really the most important
  change, and I see no reason not to do this right now.

  This is is easy; just remove the .if on PKG_DEVELOPER on line 428 of
  bsd.prefs.mk, and replace with "USE_DESTDIR?= yes".

  2) Change the current warning to be always enabled.  I see no reason
  not to do this immediately

  3) Change the code, similarly to your patch, to set BROKEN if both a)
  USE_DESTDIR=yes and b) the package is not DESTDIR-ready.  I see no
  reason not to do this right now.

  3) While leaving the option to not use DESTDIR mode, mark packages
  that aren't DESTDIR-capable BROKEN.  This is what you proposed, and I
  don't think it makes sense until (1) has been in place for a while.
  It seems punitive more than useful, as people using non-DESTDIR
  packages have their world break without getting any of the benefits.

  4) Remove the option to use other than DESTDIR mode.  Given the
  progress, I expect pretty soon the number of deficient packages will
  be really small, instead of just small.  But I don't think we've
  reached the time to do this step.
  
Also, the existing code is about PKG_DEVELOPER, and it seems that
PKG_DEVELOPER should control other options, and not be a switch itself.
But perhaps that's left over "developers should be warned about lack of
destdir support".  That makes me think that some other place is
appropriate to add the BROKEN, near where the logic is not to use
DESTDIR, even if USE_DESTDIR=yes.

What are your thoughts on when step 1 should be taken?  (I am totally ok
with you doing it today.)


Proposed change for 1 and 2.  Seems to work with light testing.

Index: bsd.prefs.mk
===================================================================
RCS file: /cvsroot/pkgsrc/mk/bsd.prefs.mk,v
retrieving revision 1.313
diff -u -p -r1.313 bsd.prefs.mk
--- bsd.prefs.mk        31 Jan 2011 08:18:44 -0000      1.313
+++ bsd.prefs.mk        6 Apr 2011 19:13:04 -0000
@@ -424,18 +424,17 @@ do-install:
        @${DO_NADA}
 .endif
 
-# PKG_DESTDIR_SUPPORT can only be one of "destdir" or "user-destdir".
-.if defined(PKG_DEVELOPER) && ${PKG_DEVELOPER} != "no"
+# As of 2011-04, the default is to use DESTDIR.
 USE_DESTDIR?=          yes
-.else
-USE_DESTDIR?=          no
-.endif
+# PKG_DESTDIR_SUPPORT can only be one of "destdir" or "user-destdir".
 PKG_DESTDIR_SUPPORT?=  # empty
 
 .if empty(PKG_DESTDIR_SUPPORT) || empty(USE_DESTDIR:M[Yy][Ee][Ss])
 .  if empty(USE_DESTDIR:M[Yy][Ee][Ss]) && empty(USE_DESTDIR:M[Nn][Oo])
 PKG_FAIL_REASON+=      "USE_DESTDIR must be either \`\`yes'' or \`\`no''"
 .  endif
+# TODO: At some point, change the next line to set BROKEN instead, causing
+# packages without DESTDIR support to fail.
 _USE_DESTDIR=          no
 .elif ${PKG_DESTDIR_SUPPORT} == "user-destdir"
 _USE_DESTDIR=          user-destdir
@@ -445,7 +444,9 @@ _USE_DESTDIR=               destdir
 PKG_FAIL_REASON+=      "PKG_DESTDIR_SUPPORT must be \`\`destdir'' or 
\`\`user-destdir''."
 .endif
 
-.if defined(PKG_DEVELOPER) && ${PKG_DEVELOPER} != "no" && 
empty(PKG_DESTDIR_SUPPORT)
+# This stanza serves to warn the user; deciding to not build
+# non-DESTDIR-capable packages when not in DESTDIR mode is above.
+.if empty(PKG_DESTDIR_SUPPORT)
 WARNINGS+=     "[bsd.prefs.mk] The package ${PKGNAME} is missing DESTDIR 
support."
 .endif

Attachment: pgpfEzXMBJH8L.pgp
Description: PGP signature



Home | Main Index | Thread Index | Old Index