cygport patches for consideration

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

cygport patches for consideration

Achim Gratz

I've prepared a branch on top of current master for your perusal:

https://repo.or.cz/cygport/rpm-style.git/shortlog/refs/heads/to-upstream


As you can see they've been battle-tested by me for quite some time already:

commit 779a7dd2fc834d45fb0f46cded647557ece17d8f (to-upstream)
Date:   Sat Apr 20 20:02:33 2013 +0200

    switch submodule gnuconfig to HTTPS protocol
   
    Live more easily with stupid firewalls...

commit fd17952c457b808e360e5ed75ffae8d9571283ad
Date:   Sun Dec 6 13:32:38 2015 +0100

    lib/pkg_pkg.cygpart: uniquify requirements after the version has been stripped off
   
    * lib/pkg_pkg.cygpart: Move the "sort -fu" command to after the
      stripping of the version part.  Otherwise some dependencies might
      get listed twice (perl_base does this sometimes).

commit 40296640fdd0951d72ae0c107f2a46bbf8111ca3
Date:   Wed Oct 3 08:56:42 2012 +0200

    support subdirectories in CPAN download URL
   
    * cygclass/perl.cygclass: Allow CPAN_AUTHOR to have an /... suffix.
      This is necessary for some modules that put the distribution files
      in some subdirectory.  Only upcase the actual author name (up until
      the first "/") and preserve case for the suffix part.  By request of
      Reini Urban, also add a variable CPAN_DIR that alternately, if set,
      will be used to concatenate as ${CPAN_AUTHOR}/${CPAN_DIR} before
      determining the CPAN download URL.  Neither variable should have a
      slash ("/") at the beginning or the end.

commit 22b3f22b92c472df80be0fdcfd3854bfd19e16f8
Date:   Sun May 18 17:52:10 2014 +0200

    pkg_info.cygport: correct search order for Perl dependencies
   
    * lib/pkg_info.cygpart: Correct search order for Perl dependencies and
      suppress auto-generation of Perl dependencies when NO_PERL_DEPS is
      defined.
   
    Dependency generation for Perl at least is too simplistic and doesn't
    take into account that some modules required or used might actually be
    optional.  It tends to generate too long dependency lists that vary
    with the Perl distributions already installed.
   
    For starters, the search order should be the reverse of
    @INC to skip dependencies that are built-in to perl already, but that
    doesn't pick up those modules that are needed with a higher version
    since only the presence of the module is detected.  Files in site_perl
    shoud never be searched since these are local installs.  Files in
    vendor_perl might be useful to check, however due to the version
    problem it is better to inject the module dpenedencies from the
    cygport file.  So skip those searches when NO_PERL_DEPS is defined,
    which it will be for auto-generated cygport files for Perl
    distributions (the information is pulled from CPAN/MetaCPAN).

commit 2e61b06df6bd5a742f92c25b50d335945e17a34b
Date:   Sat Aug 18 12:43:23 2018 +0200

    bin/cygport.in: provide all-test / almostall-test commands on CLI for symmetry with package-test / pkg-test

commit 34e6e1f2dcdf45ac4a3f9e70cae1a8290d860cd9
Date:   Fri Nov 3 21:47:54 2017 +0100

    Automatically create a test release if the release string starts with a literal "0"
   
    * lib/pkg_pkg.cygpart: Test for a literal "0" as the first character
      in the release string and make a test release if true.

commit 7f6fb93eaa9e4afe9e12bd57ebb3e8a4daa135ff
Date:   Sat Apr 8 17:00:34 2017 +0200

    Show pkg_tag in chatter
   
    lib/pkg_pkg.cygpart: inform when creating hint files for a test
    release.

commit 01e199380e32f214c2809d3dcc76f51b61de6d01
Date:   Sat Sep 17 10:07:10 2016 +0200

    lib/src_install.cygpart: correct test in make_etc_defaults, possibly show diff
   
    * lib/src_install.cygpart (make_etc_defaults): The preremove script
      only removes plain files when they match the default, so the
      postinstall script must not install files if _anything_ with the
      same name already exists.  Change the test from '-f' to '-e'.  If
      /usr/bin/diff is installed and the target is a plain file, show the
      diff to the default so the user can decide more easily what to do.


Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada
Reply | Threaded
Open this post in threaded view
|

Re: cygport patches for consideration

Brian Inglis
On 2020-04-05 12:54, Achim Gratz wrote:

> I've prepared a branch on top of current master for your perusal:
> https://repo.or.cz/cygport/rpm-style.git/shortlog/refs/heads/to-upstream
> As you can see they've been battle-tested by me for quite some time already:
> commit 779a7dd2fc834d45fb0f46cded647557ece17d8f (to-upstream)
> Date:   Sat Apr 20 20:02:33 2013 +0200
>     switch submodule gnuconfig to HTTPS protocol
>     Live more easily with stupid firewalls...
> commit fd17952c457b808e360e5ed75ffae8d9571283ad
> Date:   Sun Dec 6 13:32:38 2015 +0100
>     lib/pkg_pkg.cygpart: uniquify requirements after the version has been stripped off
>     * lib/pkg_pkg.cygpart: Move the "sort -fu" command to after the
>       stripping of the version part.  Otherwise some dependencies might
>       get listed twice (perl_base does this sometimes).
> commit 40296640fdd0951d72ae0c107f2a46bbf8111ca3
> Date:   Wed Oct 3 08:56:42 2012 +0200
>     support subdirectories in CPAN download URL
>     * cygclass/perl.cygclass: Allow CPAN_AUTHOR to have an /... suffix.
>       This is necessary for some modules that put the distribution files
>       in some subdirectory.  Only upcase the actual author name (up until
>       the first "/") and preserve case for the suffix part.  By request of
>       Reini Urban, also add a variable CPAN_DIR that alternately, if set,
>       will be used to concatenate as ${CPAN_AUTHOR}/${CPAN_DIR} before
>       determining the CPAN download URL.  Neither variable should have a
>       slash ("/") at the beginning or the end.
> commit 22b3f22b92c472df80be0fdcfd3854bfd19e16f8
> Date:   Sun May 18 17:52:10 2014 +0200
>     pkg_info.cygport: correct search order for Perl dependencies
>     * lib/pkg_info.cygpart: Correct search order for Perl dependencies and
>       suppress auto-generation of Perl dependencies when NO_PERL_DEPS is
>       defined.
>     Dependency generation for Perl at least is too simplistic and doesn't
>     take into account that some modules required or used might actually be
>     optional.  It tends to generate too long dependency lists that vary
>     with the Perl distributions already installed.
>     For starters, the search order should be the reverse of
>     @INC to skip dependencies that are built-in to perl already, but that
>     doesn't pick up those modules that are needed with a higher version
>     since only the presence of the module is detected.  Files in site_perl
>     shoud never be searched since these are local installs.  Files in
>     vendor_perl might be useful to check, however due to the version
>     problem it is better to inject the module dpenedencies from the
>     cygport file.  So skip those searches when NO_PERL_DEPS is defined,
>     which it will be for auto-generated cygport files for Perl
>     distributions (the information is pulled from CPAN/MetaCPAN).
> commit 2e61b06df6bd5a742f92c25b50d335945e17a34b
> Date:   Sat Aug 18 12:43:23 2018 +0200
>     bin/cygport.in: provide all-test / almostall-test commands on CLI for symmetry with package-test / pkg-test
> commit 34e6e1f2dcdf45ac4a3f9e70cae1a8290d860cd9
> Date:   Fri Nov 3 21:47:54 2017 +0100
>     Automatically create a test release if the release string starts with a literal "0"
>     * lib/pkg_pkg.cygpart: Test for a literal "0" as the first character
>       in the release string and make a test release if true.
> commit 7f6fb93eaa9e4afe9e12bd57ebb3e8a4daa135ff
> Date:   Sat Apr 8 17:00:34 2017 +0200
>     Show pkg_tag in chatter
>     lib/pkg_pkg.cygpart: inform when creating hint files for a test
>     release.
> commit 01e199380e32f214c2809d3dcc76f51b61de6d01
> Date:   Sat Sep 17 10:07:10 2016 +0200
>     lib/src_install.cygpart: correct test in make_etc_defaults, possibly show diff
>     * lib/src_install.cygpart (make_etc_defaults): The preremove script
>       only removes plain files when they match the default, so the
>       postinstall script must not install files if _anything_ with the
>       same name already exists.  Change the test from '-f' to '-e'.  If
>       /usr/bin/diff is installed and the target is a plain file, show the
>       diff to the default so the user can decide more easily what to do.

Capturing your commentary, especially of variables, and on effects and side
effects e.g. release string first char 0 => test release, in some doc patches
would be extremely valuable, and in keeping with the high standards set for the
existing package docs.

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
Reply | Threaded
Open this post in threaded view
|

Re: cygport patches for consideration

Yaakov Selkowitz
In reply to this post by Achim Gratz
On Sun, 2020-04-05 at 20:54 +0200, Achim Gratz wrote:

> I've prepared a branch on top of current master for your perusal:
>
> https://repo.or.cz/cygport/rpm-style.git/shortlog/refs/heads/to-upstream
>
> As you can see they've been battle-tested by me for quite some time already:
>
> commit 779a7dd2fc834d45fb0f46cded647557ece17d8f (to-upstream)
> Date:   Sat Apr 20 20:02:33 2013 +0200
>
>     switch submodule gnuconfig to HTTPS protocol
>    
>     Live more easily with stupid firewalls...
Pushed to master.

> commit fd17952c457b808e360e5ed75ffae8d9571283ad
> Date:   Sun Dec 6 13:32:38 2015 +0100
>
>     lib/pkg_pkg.cygpart: uniquify requirements after the version has been stripped off
>    
>     * lib/pkg_pkg.cygpart: Move the "sort -fu" command to after the
>       stripping of the version part.  Otherwise some dependencies might
>       get listed twice (perl_base does this sometimes).

Please no CVS style commit messages, I haven't done that in cygport for
over a decade.  Code itself looks okay though.

> commit 40296640fdd0951d72ae0c107f2a46bbf8111ca3
> Date:   Wed Oct 3 08:56:42 2012 +0200
>
>     support subdirectories in CPAN download URL
>    
>     * cygclass/perl.cygclass: Allow CPAN_AUTHOR to have an /... suffix.
>       This is necessary for some modules that put the distribution files
>       in some subdirectory.  Only upcase the actual author name (up until
>       the first "/") and preserve case for the suffix part.  By request of
>       Reini Urban, also add a variable CPAN_DIR that alternately, if set,
>       will be used to concatenate as ${CPAN_AUTHOR}/${CPAN_DIR} before
>       determining the CPAN download URL.  Neither variable should have a
>       slash ("/") at the beginning or the end.
There is no need for two variables to do the same thing.  I like
Reini's idea but let's call it CPAN_SUBDIR instead.  What about the
attached patch 0001?

> commit 22b3f22b92c472df80be0fdcfd3854bfd19e16f8
> Date:   Sun May 18 17:52:10 2014 +0200
>
>     pkg_info.cygport: correct search order for Perl dependencies
>    
>     * lib/pkg_info.cygpart: Correct search order for Perl dependencies and
>       suppress auto-generation of Perl dependencies when NO_PERL_DEPS is
>       defined.
>    
>     Dependency generation for Perl at least is too simplistic and doesn't
>     take into account that some modules required or used might actually be
>     optional.  It tends to generate too long dependency lists that vary
>     with the Perl distributions already installed.
>    
>     For starters, the search order should be the reverse of
>     @INC to skip dependencies that are built-in to perl already, but that
>     doesn't pick up those modules that are needed with a higher version
>     since only the presence of the module is detected.  Files in site_perl
>     shoud never be searched since these are local installs.  Files in
>     vendor_perl might be useful to check, however due to the version
>     problem it is better to inject the module dpenedencies from the
>     cygport file.  So skip those searches when NO_PERL_DEPS is defined,
>     which it will be for auto-generated cygport files for Perl
>     distributions (the information is pulled from CPAN/MetaCPAN).
Let's improve the auto-detection instead.  What about the attached
patch 0002?

> commit 2e61b06df6bd5a742f92c25b50d335945e17a34b
> Date:   Sat Aug 18 12:43:23 2018 +0200
>
>     bin/cygport.in: provide all-test / almostall-test commands on CLI for symmetry with package-test / pkg-test

Nak.  almostall is a deprecated alias of all, there is no need for
symmetry here.

> commit 34e6e1f2dcdf45ac4a3f9e70cae1a8290d860cd9
> Date:   Fri Nov 3 21:47:54 2017 +0100
>
>     Automatically create a test release if the release string starts with a literal "0"
>    
>     * lib/pkg_pkg.cygpart: Test for a literal "0" as the first character
>       in the release string and make a test release if true.

Nak.  There is not necessarily any correlation between a -0.* release
and whether it should be test or not.

> commit 7f6fb93eaa9e4afe9e12bd57ebb3e8a4daa135ff
> Date:   Sat Apr 8 17:00:34 2017 +0200
>
>     Show pkg_tag in chatter
>    
>     lib/pkg_pkg.cygpart: inform when creating hint files for a test
>     release

Pushed a slightly different approach to master.  Not sure what the
second part of that patch is supposed to be, however, as there is no
$pkg_testrelease variable in cygport.

> commit 01e199380e32f214c2809d3dcc76f51b61de6d01
> Date:   Sat Sep 17 10:07:10 2016 +0200
>
>     lib/src_install.cygpart: correct test in make_etc_defaults, possibly show diff
>    
>     * lib/src_install.cygpart (make_etc_defaults): The preremove script
>       only removes plain files when they match the default, so the
>       postinstall script must not install files if _anything_ with the
>       same name already exists.  Change the test from '-f' to '-e'.  If
>       /usr/bin/diff is installed and the target is a plain file, show the
>       diff to the default so the user can decide more easily what to do.
The first part is ok once the commit is reworded.  But when would a
user see the output of a postinstall script?  What would make more
sense is to have a utility akin to "rpmconf -a" on RPM-based systems
which allows the user to compare existing files with their
/etc/defaults and choose if and how to merge the differences.

--
Yaakov


0001-perl-cpan-subdir.patch (1K) Download Attachment
0002-cygport-perl-dep.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: cygport patches for consideration

Achim Gratz
Yaakov Selkowitz writes:
>>     support subdirectories in CPAN download URL
>
> There is no need for two variables to do the same thing.  I like
> Reini's idea but let's call it CPAN_SUBDIR instead.  What about the
> attached patch 0001?

Well, Reini's chimed in after he'd seen my patch and I just added that
functionality on top of what was already implemented and in use by
myself.  I guess I can change my cygport generator instead to use
CPAN_DIR when needed, but haven't got around doing so.

>>     pkg_info.cygport: correct search order for Perl dependencies
>
> Let's improve the auto-detection instead.  What about the attached
> patch 0002?

Autodetection just doesn't work, with both false positives and negatives
in spades.  Your proposed change probably produces more false negatives
with not much change in the false positive rate.  So why bother?

There are actually multiple modules on CPAN attempting that sort of
thing, one slower than the other and still none of them gets it right in
all cases.  So your chances of doing better with a shell script are
pretty much zero.  All Perl distributions come with metadata that tell
you the dependencies among them already, so I just need a way to
transfer that exactly into the cygport file (which again gets generated
from that exact metadata, not written by hand).  By way of example,
Cygwin has several packages that support Perl OO layers, of which there
are many (Moose, Mouse, Moo, Mo…) so a dependency check pulling in all
of them would end up creating a dependency chain that's at least 200
more distributions.  Note that I very deliberately do not package Moose,
which is huge.

>>     Automatically create a test release if the release string starts with a literal "0"
>
> Nak.  There is not necessarily any correlation between a -0.* release
> and whether it should be test or not.

Then there ought to be. :-)

>>     Show pkg_tag in chatter
>
> Pushed a slightly different approach to master.

I obviously like my patch better, because it keeps that line in the
output no matter what the tag is (plus colors it so it's easier to catch
a left over wrong tag when scrolling through the backlog).

> Not sure what the
> second part of that patch is supposed to be, however, as there is no
> $pkg_testrelease variable in cygport.

That's a vestigial of some other patch that was ordered before this one
before I created the branch, I think.

>>     lib/src_install.cygpart: correct test in make_etc_defaults, possibly show diff
>
> The first part is ok once the commit is reworded.  But when would a
> user see the output of a postinstall script?

When looking in setup.log.full…  this output used to go to the console,
but got axed quite some time ago.

> What would make more sense is to have a utility akin to "rpmconf -a"
> on RPM-based systems which allows the user to compare existing files
> with their /etc/defaults and choose if and how to merge the
> differences.

Sure, but that's not cygport's business, no?


Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Waldorf MIDI Implementation & additional documentation:
http://Synth.Stromeko.net/Downloads.html#WaldorfDocs
Reply | Threaded
Open this post in threaded view
|

Re: cygport patches for consideration

Jon TURNEY
On 07/04/2020 17:52, Achim Gratz wrote:
> Yaakov Selkowitz writes:
>
>>>      Automatically create a test release if the release string starts with a literal "0"
>>
>> Nak.  There is not necessarily any correlation between a -0.* release
>> and whether it should be test or not.
>
> Then there ought to be. :-)

The current package naming guidelines say that a release starting with
'0' should be used for pre-release versions. Those are not necessarily
test versions.
Reply | Threaded
Open this post in threaded view
|

Re: cygport patches for consideration

Yaakov Selkowitz
In reply to this post by Achim Gratz
On Tue, 2020-04-07 at 18:52 +0200, Achim Gratz wrote:

> Yaakov Selkowitz writes:
> > >     support subdirectories in CPAN download URL
> >
> > There is no need for two variables to do the same thing.  I like
> > Reini's idea but let's call it CPAN_SUBDIR instead.  What about the
> > attached patch 0001?
>
> Well, Reini's chimed in after he'd seen my patch and I just added that
> functionality on top of what was already implemented and in use by
> myself.  I guess I can change my cygport generator instead to use
> CPAN_DIR when needed, but haven't got around doing so.

Depending on its size, it would be nice to get this generator into
cygport's tools, and could possibly be used as the basis for other such
generators for other languages.

> > >     pkg_info.cygport: correct search order for Perl dependencies
> >
> > Let's improve the auto-detection instead.  What about the attached
> > patch 0002?
>
> Autodetection just doesn't work, with both false positives and negatives
> in spades.  Your proposed change probably produces more false negatives
> with not much change in the false positive rate.  So why bother?

This change would reduce the number of possible dependencies found by
only looking for those starting at the beginning of a line, which
should eliminate false positives from perldocs and optional deps.  So,
yes, some more false negatives, but also much less false positives.
When would there be false positives in this case?

> There are actually multiple modules on CPAN attempting that sort of
> thing, one slower than the other and still none of them gets it right in
> all cases.  So your chances of doing better with a shell script are
> pretty much zero.  All Perl distributions come with metadata that tell
> you the dependencies among them already, so I just need a way to
> transfer that exactly into the cygport file (which again gets generated
> from that exact metadata, not written by hand).  By way of example,
> Cygwin has several packages that support Perl OO layers, of which there
> are many (Moose, Mouse, Moo, Mo…) so a dependency check pulling in all
> of them would end up creating a dependency chain that's at least 200
> more distributions.  Note that I very deliberately do not package Moose,
> which is huge.
>
> > >     Automatically create a test release if the release string starts with a literal "0"
> >
> > Nak.  There is not necessarily any correlation between a -0.* release
> > and whether it should be test or not.
>
> Then there ought to be. :-)

No, not really.

> > >     Show pkg_tag in chatter
> >
> > Pushed a slightly different approach to master.
>
> I obviously like my patch better, because it keeps that line in the
> output no matter what the tag is (plus colors it so it's easier to catch
> a left over wrong tag when scrolling through the backlog).

Bikeshedding. :-)  There is an indication now of test:, which should
suffice.

> > Not sure what the
> > second part of that patch is supposed to be, however, as there is no
> > $pkg_testrelease variable in cygport.
>
> That's a vestigial of some other patch that was ordered before this one
> before I created the branch, I think.

Will ignore then.

> > >     lib/src_install.cygpart: correct test in make_etc_defaults, possibly show diff
> >
> > The first part is ok once the commit is reworded.  But when would a
> > user see the output of a postinstall script?
>
> When looking in setup.log.full…  this output used to go to the console,
> but got axed quite some time ago.

Most people aren't going to check the log for that, nor does the log
allow them to do anything about it.

> > What would make more sense is to have a utility akin to "rpmconf -a"
> > on RPM-based systems which allows the user to compare existing files
> > with their /etc/defaults and choose if and how to merge the
> > differences.
>
> Sure, but that's not cygport's business, no?

No, this would be something separate, or possibly part of cygutils.
But's it's not the postinstall's business either AFAIAC.

--
Yaakov


Reply | Threaded
Open this post in threaded view
|

Re: cygport patches for consideration

Achim Gratz
Yaakov Selkowitz writes:
>> I guess I can change my cygport generator instead to use
>> CPAN_DIR when needed, but haven't got around doing so.
>
> Depending on its size, it would be nice to get this generator into
> cygport's tools, and could possibly be used as the basis for other such
> generators for other languages.

It does a few other things before it gets to generating a cygport file.
It'd probably be a day or two of excising the cygport generation
facility, but again I'd need a round tuit.  But then again who knows
what we might have time for unexpectedly…

> This change would reduce the number of possible dependencies found by
> only looking for those starting at the beginning of a line, which
> should eliminate false positives from perldocs and optional deps.  So,
> yes, some more false negatives, but also much less false positives.
> When would there be false positives in this case?

I haven't tested it, but it seems that you would pick up conditional
imports that employ the "if" pragmatic module (which is the canonical
way of doing conditional imports).  Short of special-casing the more
common uses of these you're left holding the bag on these anyway since
you simply can't know whether the condition is true without evaluating
the scope.  On the other hand you're now prone to skip imports in BEGIN
blocks, since these would mostly be indented.

>> When looking in setup.log.full…  this output used to go to the console,
>> but got axed quite some time ago.
>
> Most people aren't going to check the log for that, nor does the log
> allow them to do anything about it.

Well, at least it lets them know _if_ they care.

>> > What would make more sense is to have a utility akin to "rpmconf -a"
>> > on RPM-based systems which allows the user to compare existing files
>> > with their /etc/defaults and choose if and how to merge the
>> > differences.
>>
>> Sure, but that's not cygport's business, no?
>
> No, this would be something separate, or possibly part of cygutils.
> But's it's not the postinstall's business either AFAIAC.

That's the only good place we have at the moment.  I've built it into
base-files three and a half years ago in case you're wondering.


Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
Reply | Threaded
Open this post in threaded view
|

Re: cygport patches for consideration

Achim Gratz
In reply to this post by Yaakov Selkowitz
Yaakov Selkowitz writes:
[…]

I've reworked the series according to your comments, force-pushed to the
same branch:

https://repo.or.cz/cygport/rpm-style.git/shortlog/refs/heads/to-upstream


Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds
Reply | Threaded
Open this post in threaded view
|

Re: cygport patches for consideration

Yaakov Selkowitz
On Sun, 2020-05-10 at 10:58 +0200, Achim Gratz wrote:
> I've reworked the series according to your comments, force-pushed to the
> same branch:
>
> https://repo.or.cz/cygport/rpm-style.git/shortlog/refs/heads/to-upstream

Merged, but changed NO_PERL_DEPS to PERL_NO_VENDOR_DEPS.

--
Yaakov