[PATCH] setup -e, --separate-src-dirs option

[PATCH] setup -e, --separate-src-dirs option

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 388 bytes --]

Many -src packages install files in /usr/src which have no 
PACKAGE[-VERSION] prefix or substring in file name. This makes it 
difficult to maintain or cleanup larger /usr/src directories.

The attached experimental patch for setup.exe adds option  -e, 
--separate-src-dirs. If specified, each PACKAGE-VERSION-src.tar.bz2 is 
installed below /usr/src/PACKAGE-VERSION instead.

Christian


[-- Attachment #2: setup-separate-src-dirs.patch --]
[-- Type: text/x-patch, Size: 1558 bytes --]

2011-12-14  Christian Franke  <franke@computer.org>

	* install.cc: New option variable SeparateSrcDirs.
	(installOne): Append PACKAGE-VERSION/ to /usr/src/
	if -e, --separate-src-dirs is set.

diff --git a/install.cc b/install.cc
index 5e3331a..638d67a 100644
--- a/install.cc
+++ b/install.cc
@@ -75,6 +75,9 @@ static int package_bytes = 0;
 static BoolOption NoReplaceOnReboot (false, 'r', "no-replaceonreboot",
 				     "Disable replacing in-use files on next "
 				     "reboot.");
+static BoolOption SeparateSrcDirs (false, 'e', "separate-src-dirs",
+				   "Install source packages in separate "
+				   "directories /usr/src/PACKAGE-VERSION.");
 
 struct std_dirs_t {
   const char *name;
@@ -740,9 +743,26 @@ do_install_thread (HINSTANCE h, HWND owner)
        i != sourceinstall_q.end (); ++i)
   {
     packagemeta & pkg = **i;
+
+    std::string prefix = "/usr/src/";
+    if (SeparateSrcDirs)
+      {
+	/* Install PACKAGE-VERSION-src.tar.bz2 contents in directory
+	   /usr/src/PACKAGE-VERSION .  */
+	const char *base = pkg.desired.sourcePackage().source()->Base();
+	if (base)
+	  {
+	    int len = strlen (base);
+	    if (len > 4 && ! strcmp (base + len - 4, "-src"))
+	      len -= 4;
+	    prefix.append(base, len);
+	    prefix += '/';
+	  }
+      }
+
     myInstaller.installOne (pkg, pkg.desired.sourcePackage(),
                             *pkg.desired.sourcePackage().source(),
-                            "cygfile://", "/usr/src/", owner);
+                            "cygfile://", prefix, owner);
   }
 
   if (rebootneeded)

Re: [PATCH] setup -e, --separate-src-dirs option

[Christopher Faylor]

On Wed, Dec 14, 2011 at 10:16:40PM +0100, Christian Franke wrote:
>Many -src packages install files in /usr/src which have no 
>PACKAGE[-VERSION] prefix or substring in file name. This makes it 
>difficult to maintain or cleanup larger /usr/src directories.
>
>The attached experimental patch for setup.exe adds option  -e, 
>--separate-src-dirs. If specified, each PACKAGE-VERSION-src.tar.bz2 is 
>installed below /usr/src/PACKAGE-VERSION instead.

If this is really desirable behavior then shouldn't we ask package
maintainers to fix their packages?  I don't see why setup.exe should
have to know about this.

cgf

Re: [PATCH] setup -e, --separate-src-dirs option

[Christian Franke]

Christopher Faylor wrote:
> On Wed, Dec 14, 2011 at 10:16:40PM +0100, Christian Franke wrote:
>> Many -src packages install files in /usr/src which have no
>> PACKAGE[-VERSION] prefix or substring in file name. This makes it
>> difficult to maintain or cleanup larger /usr/src directories.
>>
>> The attached experimental patch for setup.exe adds option  -e,
>> --separate-src-dirs. If specified, each PACKAGE-VERSION-src.tar.bz2 is
>> installed below /usr/src/PACKAGE-VERSION instead.
> If this is really desirable behavior then shouldn't we ask package
> maintainers to fix their packages?  I don't see why setup.exe should
> have to know about this.
>

The patch is a pragmatic approach which works with all existing 
packages. It doesn't break anything existing. It would last long time 
until all src tarballs would be fixed.

It is actually difficult to guess the origin of some source files. For 
example:

/usr/src/0.19-data-auto-imports.patch (from flexdll-0.26-1)
/usr/src/blacklist.txt (from ca-cerficates-*)
/usr/src/config-rpath.patch (from some gcc-* ?)

Christian

Re: [PATCH] setup -e, --separate-src-dirs option

[Corinna Vinschen]

On Dec 15 07:38, Christian Franke wrote:
> Christopher Faylor wrote:
> >On Wed, Dec 14, 2011 at 10:16:40PM +0100, Christian Franke wrote:
> >>Many -src packages install files in /usr/src which have no
> >>PACKAGE[-VERSION] prefix or substring in file name. This makes it
> >>difficult to maintain or cleanup larger /usr/src directories.
> >>
> >>The attached experimental patch for setup.exe adds option  -e,
> >>--separate-src-dirs. If specified, each PACKAGE-VERSION-src.tar.bz2 is
> >>installed below /usr/src/PACKAGE-VERSION instead.
> >If this is really desirable behavior then shouldn't we ask package
> >maintainers to fix their packages?  I don't see why setup.exe should
> >have to know about this.
> >
> 
> The patch is a pragmatic approach which works with all existing
> packages. It doesn't break anything existing. It would last long
> time until all src tarballs would be fixed.
> 
> It is actually difficult to guess the origin of some source files.
> For example:
> 
> /usr/src/0.19-data-auto-imports.patch (from flexdll-0.26-1)
> /usr/src/blacklist.txt (from ca-cerficates-*)
> /usr/src/config-rpath.patch (from some gcc-* ?)

I'm wondering if what you did in your patch shouldn't be just the
default behaviour.  No -e option.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] setup -e, --separate-src-dirs option

[marco atzeri]

On 12/15/2011 9:56 AM, Corinna Vinschen wrote:
> I'm wondering if what you did in your patch shouldn't be just the
> default behaviour.  No -e option.

not a bad idea.

+1

almost all the cygwin source packages have no subdir structure, 
including all the ones packaged with cygport.

Marco

Re: [PATCH] setup -e, --separate-src-dirs option

[Ken Brown]

On 12/15/2011 7:58 AM, marco atzeri wrote:
> On 12/15/2011 9:56 AM, Corinna Vinschen wrote:
>> I'm wondering if what you did in your patch shouldn't be just the
>> default behaviour. No -e option.
>
> not a bad idea.
>
> +1
>
> almost all the cygwin source packages have no subdir structure,
> including all the ones packaged with cygport.

+1

I've often downloaded sources and found a bunch of patches in /usr/src, 
with no way of guessing from the filenames which package they belong to.

Ken

Re: [PATCH] setup -e, --separate-src-dirs option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 520 bytes --]

On 12/15/2011 01:56 AM, Corinna Vinschen wrote:
>>>> The attached experimental patch for setup.exe adds option  -e,
>>>> --separate-src-dirs. If specified, each PACKAGE-VERSION-src.tar.bz2 is
>>>> installed below /usr/src/PACKAGE-VERSION instead.
> 
> I'm wondering if what you did in your patch shouldn't be just the
> default behaviour.  No -e option.

+1 to doing this by default, with no option.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [PATCH] setup -e, --separate-src-dirs option

[Christopher Faylor]

On Thu, Dec 15, 2011 at 07:38:14AM +0100, Christian Franke wrote:
>Christopher Faylor wrote:
>> On Wed, Dec 14, 2011 at 10:16:40PM +0100, Christian Franke wrote:
>>> Many -src packages install files in /usr/src which have no
>>> PACKAGE[-VERSION] prefix or substring in file name. This makes it
>>> difficult to maintain or cleanup larger /usr/src directories.
>>>
>>> The attached experimental patch for setup.exe adds option  -e,
>>> --separate-src-dirs. If specified, each PACKAGE-VERSION-src.tar.bz2 is
>>> installed below /usr/src/PACKAGE-VERSION instead.
>> If this is really desirable behavior then shouldn't we ask package
>> maintainers to fix their packages?  I don't see why setup.exe should
>> have to know about this.
>>
>
>The patch is a pragmatic approach which works with all existing 
>packages. It doesn't break anything existing. It would last long time 
>until all src tarballs would be fixed.
>
>It is actually difficult to guess the origin of some source files. For 
>example:
>
>/usr/src/0.19-data-auto-imports.patch (from flexdll-0.26-1)
>/usr/src/blacklist.txt (from ca-cerficates-*)
>/usr/src/config-rpath.patch (from some gcc-* ?)

That's not difficult.  It's trivial to figure out where these files
came from.

I still don't understand the need for this.  If everyone thinks it's
a good idea than why don't we eschew code bloat and make package
developers use this technique.  Otherwise, unless you inspect the
source files, you'll be adding a separate layer of directory to
/usr/src for packages that don't need it.

cgf
layer of directory for 

Re: [PATCH] setup -e, --separate-src-dirs option

[Christian Franke]

Christopher Faylor wrote:
> On Thu, Dec 15, 2011 at 07:38:14AM +0100, Christian Franke wrote:
>> It is actually difficult to guess the origin of some source files. For
>> example:
>>
>> /usr/src/0.19-data-auto-imports.patch (from flexdll-0.26-1)
>> /usr/src/blacklist.txt (from ca-cerficates-*)
>> /usr/src/config-rpath.patch (from some gcc-* ?)
> That's not difficult.  It's trivial to figure out where these files
> came from.

Even if this is the case (it IMO isn't), there is at least one other 
drawback. Due to missing version numbers in some names, it is impossible 
to keep several versions of the same package in /usr/src.


>
> I still don't understand the need for this.

Please note that I suggested to *keep* the old behavior and let the 
*user* opt-in for the new one. I actually missed that feature since I 
started to use Cygwin many years ago.


>    If everyone thinks it's
> a good idea than why don't we eschew code bloat and make package
> developers use this technique.  Otherwise, unless you inspect the
> source files, you'll be adding a separate layer of directory to
> /usr/src for packages that don't need it.

Another more complex approach would be: Examine the 
PACKAGE-VERSION-src.tar.bz file first. If all top level names start with 
(or contain?) PACKAGE-VERSION, install in /usr/src, else install in 
/usr/src/PACKAGE-VERSION. But I will not provide such a patch this year :-)

Christian

Re: [PATCH] setup -e, --separate-src-dirs option

[Corinna Vinschen]

On Dec 15 13:10, Christopher Faylor wrote:
> On Thu, Dec 15, 2011 at 07:38:14AM +0100, Christian Franke wrote:
> >Christopher Faylor wrote:
> >> On Wed, Dec 14, 2011 at 10:16:40PM +0100, Christian Franke wrote:
> >>> Many -src packages install files in /usr/src which have no
> >>> PACKAGE[-VERSION] prefix or substring in file name. This makes it
> >>> difficult to maintain or cleanup larger /usr/src directories.
> >>>
> >>> The attached experimental patch for setup.exe adds option  -e,
> >>> --separate-src-dirs. If specified, each PACKAGE-VERSION-src.tar.bz2 is
> >>> installed below /usr/src/PACKAGE-VERSION instead.
> >> If this is really desirable behavior then shouldn't we ask package
> >> maintainers to fix their packages?  I don't see why setup.exe should
> >> have to know about this.
> >>
> >
> >The patch is a pragmatic approach which works with all existing 
> >packages. It doesn't break anything existing. It would last long time 
> >until all src tarballs would be fixed.
> >
> >It is actually difficult to guess the origin of some source files. For 
> >example:
> >
> >/usr/src/0.19-data-auto-imports.patch (from flexdll-0.26-1)
> >/usr/src/blacklist.txt (from ca-cerficates-*)
> >/usr/src/config-rpath.patch (from some gcc-* ?)
> 
> That's not difficult.  It's trivial to figure out where these files
> came from.
> 
> I still don't understand the need for this.  If everyone thinks it's
> a good idea than why don't we eschew code bloat and make package
> developers use this technique.  Otherwise, unless you inspect the
> source files, you'll be adding a separate layer of directory to
> /usr/src for packages that don't need it.

I don't see a problem with that.  I don't think it's the better approach
to wait for all package maintainers to create new source packages.  It's
much easier to do it once and for all in setup.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] setup -e, --separate-src-dirs option

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 315 bytes --]

Corinna Vinschen wrote:
> I'm wondering if what you did in your patch shouldn't be just the
> default behaviour.  No -e option.
>

Of course :-)

+1

Thanks for all +1 feedbacks.
New patch attached, function name in changelog fixed.
The nullptr check for Base() return value might not be really needed.

Christian


[-- Attachment #2: setup-separate-src-dirs-2.patch --]
[-- Type: text/x-patch, Size: 1074 bytes --]

2011-12-15  Christian Franke  <franke@computer.org>

	* install.cc (do_install_thread): Install src packages
	in /usr/src/PACKAGE-VERSION instead of /usr/src.

diff --git a/install.cc b/install.cc
index 5e3331a..89e8ce0 100644
--- a/install.cc
+++ b/install.cc
@@ -740,9 +740,23 @@ do_install_thread (HINSTANCE h, HWND owner)
        i != sourceinstall_q.end (); ++i)
   {
     packagemeta & pkg = **i;
+
+    std::string prefix = "/usr/src/";
+    /* Install PACKAGE-VERSION-src.tar.bz2 contents in directory
+       /usr/src/PACKAGE-VERSION .  */
+    const char *base = pkg.desired.sourcePackage().source()->Base();
+    if (base)
+      {
+	int len = strlen (base);
+	if (len > 4 && ! strcmp (base + len - 4, "-src"))
+	  len -= 4;
+	prefix.append(base, len);
+	prefix += '/';
+      }
+
     myInstaller.installOne (pkg, pkg.desired.sourcePackage(),
                             *pkg.desired.sourcePackage().source(),
-                            "cygfile://", "/usr/src/", owner);
+                            "cygfile://", prefix, owner);
   }
 
   if (rebootneeded)

Re: [PATCH] setup -e, --separate-src-dirs option

[Christopher Faylor]

On Thu, Dec 15, 2011 at 08:02:12PM +0100, Christian Franke wrote:
>Christopher Faylor wrote:
>> On Thu, Dec 15, 2011 at 07:38:14AM +0100, Christian Franke wrote:
>>> It is actually difficult to guess the origin of some source files. For
>>> example:
>>>
>>> /usr/src/0.19-data-auto-imports.patch (from flexdll-0.26-1)
>>> /usr/src/blacklist.txt (from ca-cerficates-*)
>>> /usr/src/config-rpath.patch (from some gcc-* ?)
>> That's not difficult.  It's trivial to figure out where these files
>> came from.
>
>Even if this is the case (it IMO isn't), there is at least one other 
>drawback.

I don't understand your opinion.  Run "tar vtjf" over all of the source
.tar.bz2 files and you can figure out where things came from.  If we decide
that this is necessary then, that can be done once on sourceware for all
.tar.bz2 files and the files can be changed.

>Due to missing version numbers in some names, it is impossible 
>to keep several versions of the same package in /usr/src.

What does that have to do with the issue?  If everybody thinks that
packages should all extract to a versioned subdirectory then that should
just be what packages do without help from setup.exe.

>>I still don't understand the need for this.
>
>Please note that I suggested to *keep* the old behavior and let the
>*user* opt-in for the new one.  I actually missed that feature since I
>started to use Cygwin many years ago.

There seems to be a communication disconnect here.  I'm not saying that
it isn't a great idea for every package to install in it's own special
subdirectory.  I am saying that if everyone agrees that it's a good idea
then we can enforce that for source packages.  Whether you have disliked
the current behavior for years is irrelevant.  I think you're right and
the "dump everything in the current directory" behavior of most of the
current packages has bugged me too.

>>    If everyone thinks it's
>> a good idea than why don't we eschew code bloat and make package
>> developers use this technique.  Otherwise, unless you inspect the
>> source files, you'll be adding a separate layer of directory to
>> /usr/src for packages that don't need it.
>
>Another more complex approach would be: Examine the 
>PACKAGE-VERSION-src.tar.bz file first. If all top level names start with 
>(or contain?) PACKAGE-VERSION, install in /usr/src, else install in 
>/usr/src/PACKAGE-VERSION. But I will not provide such a patch this year :-)

Hmm.  Or maybe the disconnect was running paragraph by paragraph.  You
at least seemed to understand that there would be an issue with packages
that already install to a directory.

The bottom line is that I'm arguing against adding more code to
setup.exe which would need to be maintained when a much simpler solution
could be enforced that requires no setup.exe code change.

cgf

Re: [PATCH] setup -e, --separate-src-dirs option

[Christopher Faylor]

On Thu, Dec 15, 2011 at 08:19:12PM +0100, Corinna Vinschen wrote:
>On Dec 15 13:10, Christopher Faylor wrote:
>> On Thu, Dec 15, 2011 at 07:38:14AM +0100, Christian Franke wrote:
>> >Christopher Faylor wrote:
>> >> On Wed, Dec 14, 2011 at 10:16:40PM +0100, Christian Franke wrote:
>> >>> Many -src packages install files in /usr/src which have no
>> >>> PACKAGE[-VERSION] prefix or substring in file name. This makes it
>> >>> difficult to maintain or cleanup larger /usr/src directories.
>> >>>
>> >>> The attached experimental patch for setup.exe adds option  -e,
>> >>> --separate-src-dirs. If specified, each PACKAGE-VERSION-src.tar.bz2 is
>> >>> installed below /usr/src/PACKAGE-VERSION instead.
>> >> If this is really desirable behavior then shouldn't we ask package
>> >> maintainers to fix their packages?  I don't see why setup.exe should
>> >> have to know about this.
>> >>
>> >
>> >The patch is a pragmatic approach which works with all existing 
>> >packages. It doesn't break anything existing. It would last long time 
>> >until all src tarballs would be fixed.
>> >
>> >It is actually difficult to guess the origin of some source files. For 
>> >example:
>> >
>> >/usr/src/0.19-data-auto-imports.patch (from flexdll-0.26-1)
>> >/usr/src/blacklist.txt (from ca-cerficates-*)
>> >/usr/src/config-rpath.patch (from some gcc-* ?)
>> 
>> That's not difficult.  It's trivial to figure out where these files
>> came from.
>> 
>> I still don't understand the need for this.  If everyone thinks it's
>> a good idea than why don't we eschew code bloat and make package
>> developers use this technique.  Otherwise, unless you inspect the
>> source files, you'll be adding a separate layer of directory to
>> /usr/src for packages that don't need it.
>
>I don't see a problem with that.  I don't think it's the better approach
>to wait for all package maintainers to create new source packages.  It's
>much easier to do it once and for all in setup.

I think it's lame to have something like, e.g.,

/usr/src/openssh-5.6p1-2/openssh-5.6p1-2
/usr/src/binutils-2.22.51-1/binutils-2.22.51-1

sitting in my /usr/src.  I find that nearly as objectionable as having
files littered in /usr/src.

If this is enforced we can't wait for package maintainers to create new
source packages.  If they create new source packages with uniquely named
directories then they will fall prey to the above.

If everyone thinks this is the way to go then I can change all of the
source files in ~release and people can change their procedures going
forward.

cgf

Re: [PATCH] setup -e, --separate-src-dirs option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2744 bytes --]

On 12/15/2011 01:59 PM, Christopher Faylor wrote:
> I think it's lame to have something like, e.g.,
> 
> /usr/src/openssh-5.6p1-2/openssh-5.6p1-2
> /usr/src/binutils-2.22.51-1/binutils-2.22.51-1
> 
> sitting in my /usr/src.  I find that nearly as objectionable as having
> files littered in /usr/src.

Agreed.  So right now, some packages use versioned subdirs, others
don't.  Your argument is that it's easier to enforce consistency on the
existing tarballs than it is to make setup.exe have a special case code,
and that new package versions should follow the conventions.

> 
> If this is enforced we can't wait for package maintainers to create new
> source packages.  If they create new source packages with uniquely named
> directories then they will fall prey to the above.
> 
> If everyone thinks this is the way to go then I can change all of the
> source files in ~release and people can change their procedures going
> forward.

If you want to go with the tarball repackaging instead of setup.exe
hack, then let's first fix cygport to generate versioned subdirs (as
that will knock out a large percentage of non-compliant packages).
Conversely, if we hack setup.exe, then the hack has to avoid creating
double-versioned directories, and only insert the versioning if one is
not already present in the existing tarball (the patches proposed to
date have not done that, but it also doesn't seem to hard to further
improve the patch).

I'm fine with either approach (minimal work for me as a packager either
way - either the packaging guidelines become stricter, but I comply the
next time I repackage with the updated cygport, or they stay loose;
meanwhile, before I repackage, the problem is already taken care of on
my behalf, whether by repackaging existing -src tarballs or by hacking
setup.exe, neither of which requires me to repackage any sooner than I'm
ready).  I care more about reaching our end goal - no patch files dumped
in the top-level /usr/src, just one-level versioned directories.

Personally, I think hacking setup.exe is less time-consuming to provide
the hack, but more compute-intensive, as the work is replicated on every
machine where setup.exe is installed, as well as delayed implementation,
as not everyone updates setup.exe right away; while repacking existing
tarballs has a bigger up-front cost for the person doing the repacking,
but provides a more efficient and instant downstream effect.  That, and
repacking seems fairly easy to automate.  I'm now 75-25 in favor of
cgf's proposed approach of repacking things and leaving setup.exe alone.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [PATCH] setup -e, --separate-src-dirs option

[Christian Franke]

Christopher Faylor wrote:
> The bottom line is that I'm arguing against adding more code to
> setup.exe which would need to be maintained when a much simpler solution
> could be enforced that requires no setup.exe code change.

Fine with me - provided that the much simpler(?) solution will actually 
be available before time_t wraps around :-)

Christian

Re: [PATCH] setup -e, --separate-src-dirs option

[Corinna Vinschen]

On Dec 15 14:13, Eric Blake wrote:
> On 12/15/2011 01:59 PM, Christopher Faylor wrote:
> > I think it's lame to have something like, e.g.,
> > 
> > /usr/src/openssh-5.6p1-2/openssh-5.6p1-2
> > /usr/src/binutils-2.22.51-1/binutils-2.22.51-1
> > 
> > sitting in my /usr/src.  I find that nearly as objectionable as having
> > files littered in /usr/src.

I don't like /usr/src/binutils-2.22.51-1/binutils-2.22.51-1 either, but
it's much less objectionable than having /usr/src littered with the
content of package files.  Especially given the fact that most(?)
packages today are packed using cygport which uses a flat file
structure.  And, I never liked the rpm way to put everything into
/usr/src/SOURCES or ~/rpmbuild/SOURCES either.  In the end, a useless
duplication of the source dir is easier to live with, IMO.

> Personally, I think hacking setup.exe is less time-consuming to provide
> the hack, but more compute-intensive, as the work is replicated on every
> machine where setup.exe is installed, as well as delayed implementation,
> as not everyone updates setup.exe right away; while repacking existing
> tarballs has a bigger up-front cost for the person doing the repacking,
> but provides a more efficient and instant downstream effect.  That, and
> repacking seems fairly easy to automate.  I'm now 75-25 in favor of
> cgf's proposed approach of repacking things and leaving setup.exe alone.

I think that computing time is negligible.  The idea to have the logic
in a single place, setup, is much more convincing to me than to enforce
a specific way to pack source packages.  Also, compare a 15 lines patch
against the hassle to change 1800 soyurce packages *and* to enforce a
new package method on all maintainers.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] setup -e, --separate-src-dirs option

[Christopher Faylor]

On Fri, Dec 16, 2011 at 10:59:25AM +0100, Corinna Vinschen wrote:
>On Dec 15 14:13, Eric Blake wrote:
>> On 12/15/2011 01:59 PM, Christopher Faylor wrote:
>> > I think it's lame to have something like, e.g.,
>> > 
>> > /usr/src/openssh-5.6p1-2/openssh-5.6p1-2
>> > /usr/src/binutils-2.22.51-1/binutils-2.22.51-1
>> > 
>> > sitting in my /usr/src.  I find that nearly as objectionable as having
>> > files littered in /usr/src.
>
>I don't like /usr/src/binutils-2.22.51-1/binutils-2.22.51-1 either, but
>it's much less objectionable than having /usr/src littered with the
>content of package files.  Especially given the fact that most(?)
>packages today are packed using cygport which uses a flat file
>structure.  And, I never liked the rpm way to put everything into
>/usr/src/SOURCES or ~/rpmbuild/SOURCES either.  In the end, a useless
>duplication of the source dir is easier to live with, IMO.

We're all in violent agreement that putting files directly in /usr/src
is bad.  This doesn't have to be an either/or situation, though.

>> Personally, I think hacking setup.exe is less time-consuming to provide
>> the hack, but more compute-intensive, as the work is replicated on every
>> machine where setup.exe is installed, as well as delayed implementation,
>> as not everyone updates setup.exe right away; while repacking existing
>> tarballs has a bigger up-front cost for the person doing the repacking,
>> but provides a more efficient and instant downstream effect.  That, and
>> repacking seems fairly easy to automate.  I'm now 75-25 in favor of
>> cgf's proposed approach of repacking things and leaving setup.exe alone.
>
>I think that computing time is negligible.  The idea to have the logic
>in a single place, setup, is much more convincing to me than to enforce
>a specific way to pack source packages.  Also, compare a 15 lines patch
>against the hassle to change 1800 soyurce packages *and* to enforce a
>new package method on all maintainers.

No need for misleading vividness.  If it was decided to change all of
the packages then the immediate impact would be on me.  I can easily
make this change to all of the packages with a few lines of bash.

After that, going forward, package maintainers would need to create
proper subdirectories.  I do that for my packages and apparently you do
for yours too.  That *was* the recommended method for creating packages
at one point.  And, as suggested, I don't see why cygport couldn't do
that automatically since the consensus is that it's a good idea.

Also, it's a very minor thing but if this patch were adopted then the
contents of a source package would not precisely match what shows up at
http://cygwin.com/packages/ .  There would be no subdirectory displayed
there.

If it is decided that asking package maintainers to change their
behaviors is too big a deal then another alternative would be to make a
"package mover" which package uploaders could run which modifies source
packages automatically before moving them to the release directory on
sourceware.  I have also been thinking for a long time that there should
be a sanity checker which could be run before installing in ~release.
That would eliminate the "upset error" email to cygwin-apps that you, I,
or Yaakov has to send when upset finds a problem later.

cgf

Re: [PATCH] setup -e, --separate-src-dirs option

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]

Corinna Vinschen wrote:
> On Dec 15 14:13, Eric Blake wrote:
>> On 12/15/2011 01:59 PM, Christopher Faylor wrote:
>>> I think it's lame to have something like, e.g.,
>>>
>>> /usr/src/openssh-5.6p1-2/openssh-5.6p1-2
>>> /usr/src/binutils-2.22.51-1/binutils-2.22.51-1
>>>
>>> sitting in my /usr/src.  I find that nearly as objectionable as having
>>> files littered in /usr/src.
> I don't like /usr/src/binutils-2.22.51-1/binutils-2.22.51-1 either, but
> it's much less objectionable than having /usr/src littered with the
> content of package files.

Attached is a new patch which takes a different approach.
It handles both cases on the fly during installation.

/usr/src/binutils-2.22.51-1/* will be installed as before, but
/usr/src/blacklist.txt will be installed as 
/usr/src/ca-certificates-1.78-1/blacklist.txt

This would break directory structure for packages which contain both, a 
PACKAGE-VERSION subdir and toplevel files. Such packages might exist or 
not. If yes, these should probably be fixed first.

Yes, I agree that this is a hack. But someone might agree that this is a 
useful hack :-)

Christian


[-- Attachment #2: setup-separate-src-dirs-3.patch --]
[-- Type: text/x-patch, Size: 3190 bytes --]

2011-12-16  Christian Franke  <franke@computer.org>

	* install.cc (installOne): Ensure that src package
	contents is installed below /usr/src/PACKAGE-VERSION.

diff --git a/install.cc b/install.cc
index 5e3331a..ad9e43d 100644
--- a/install.cc
+++ b/install.cc
@@ -401,6 +401,7 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
      filename of each file that was unpacked.  */
 
   io_stream *lst = NULL;
+  std::string srcSubDir;
   if (ver.Type () == package_binary)
     {
       std::string lstfn = "cygfile:///etc/setup/" + pkgm.name + ".lst.gz";
@@ -422,6 +423,17 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
             }
         }
     }
+  else
+    {
+      /* Extract PACKAGE-VERSION from src archive name.  */
+      const char *base = source.Base ();
+      int len = (base ? strlen (base) : 0);
+      if (len > 4 && ! strcmp (base + len - 4, "-src"))
+	{
+	  srcSubDir.assign (base, len - 4);
+	  srcSubDir += '/';
+	}
+   }
 
   bool error_in_this_package = false;
   bool ignoreInUseErrors = unattended_mode;
@@ -430,12 +442,22 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
   package_bytes = source.size;
   log (LOG_PLAIN) << "Extracting from " << source.Cached () << endLog;
 
+  std::string fullPrefixPath = prefixPath;
   std::string fn;
   while ((fn = tarstream->next_file_name ()).size ())
     {
-      std::string canonicalfn = prefixPath + fn;
+      if (! srcSubDir.empty ())
+	{
+	  /* Ensure that src archive contents is installed below
+	     /usr/src/PACKAGE-VERSION.  */
+	  fullPrefixPath = prefixPath;
+	  if (strncmp (fn.c_str (), srcSubDir.c_str (), srcSubDir.size ()))
+	    fullPrefixPath += srcSubDir;
+	}
+
+      std::string canonicalfn = fullPrefixPath + fn;
       Progress.SetText3 (canonicalfn.c_str ());
-      log (LOG_BABBLE) << "Installing file " << prefixURL << prefixPath
+      log (LOG_BABBLE) << "Installing file " << prefixURL << fullPrefixPath
           << fn << endLog;
       if (lst)
         {
@@ -447,7 +469,7 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
 
       bool firstIteration = true;
       int extract_error = 0;
-      while ((extract_error = archive::extract_file (tarstream, prefixURL, prefixPath)) != 0)
+      while ((extract_error = archive::extract_file (tarstream, prefixURL, fullPrefixPath)) != 0)
         {
           switch (extract_error)
             {
@@ -483,11 +505,11 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
                     ++errors;
                     error_in_this_package = true;
                     log (LOG_PLAIN) << "Not replacing in-use file " << prefixURL
-                                    << prefixPath << fn << endLog;
+                                    << fullPrefixPath << fn << endLog;
                   }
                 else
                   {
-                    error_in_this_package = extract_replace_on_reboot(tarstream, prefixURL, prefixPath, fn);
+                    error_in_this_package = extract_replace_on_reboot(tarstream, prefixURL, fullPrefixPath, fn);
                   }
               }
               break;

Re: [PATCH] setup -e, --separate-src-dirs option

[Peter Rosin]

Christian Franke skrev 2011-12-16 21:25:
> Corinna Vinschen wrote:
>> On Dec 15 14:13, Eric Blake wrote:
>>> On 12/15/2011 01:59 PM, Christopher Faylor wrote:
>>>> I think it's lame to have something like, e.g.,
>>>>
>>>> /usr/src/openssh-5.6p1-2/openssh-5.6p1-2
>>>> /usr/src/binutils-2.22.51-1/binutils-2.22.51-1
>>>>
>>>> sitting in my /usr/src.  I find that nearly as objectionable as having
>>>> files littered in /usr/src.
>> I don't like /usr/src/binutils-2.22.51-1/binutils-2.22.51-1 either, but
>> it's much less objectionable than having /usr/src littered with the
>> content of package files.
> 
> Attached is a new patch which takes a different approach.
> It handles both cases on the fly during installation.
> 
> /usr/src/binutils-2.22.51-1/* will be installed as before, but
> /usr/src/blacklist.txt will be installed as /usr/src/ca-certificates-1.78-1/blacklist.txt
> 
> This would break directory structure for packages which contain both, a PACKAGE-VERSION subdir and toplevel files. Such packages might exist or not. If yes, these should probably be fixed first.
> 
> Yes, I agree that this is a hack. But someone might agree that this is a useful hack :-)

If we don't play games like this in setup.exe, it is still possible
to create a structure beneath /usr/src/foo with different packages
with some foo relation. With an automatic remapping in setup, that is
no longer possible. Not that I know of any such structure at this time,
but something to consider...

Cheers,
Peter