public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
* cygport
@ 2022-01-10 18:10 Achim Gratz
  2022-03-14 19:06 ` cygport Jon Turney
  2022-09-15 17:45 ` cygport Jon Turney
  0 siblings, 2 replies; 9+ messages in thread
From: Achim Gratz @ 2022-01-10 18:10 UTC (permalink / raw)
  To: cygwin-apps


I've rebased the remaining patches on my to-upstream branch onto the
current release of cygport:

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

Note that some of these are required to correctly build and distribute
Perl and its modules for Cygwin (which is one reason I carry these for
several years now).


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

SD adaptation for Waldorf Blofeld V1.15B11:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: cygport
  2022-01-10 18:10 cygport Achim Gratz
@ 2022-03-14 19:06 ` Jon Turney
  2022-03-14 20:12   ` cygport Achim Gratz
  2022-03-27 13:22   ` cygport Achim Gratz
  2022-09-15 17:45 ` cygport Jon Turney
  1 sibling, 2 replies; 9+ messages in thread
From: Jon Turney @ 2022-03-14 19:06 UTC (permalink / raw)
  To: Achim Gratz, cygwin-apps

On 10/01/2022 18:10, Achim Gratz wrote:
> 
> I've rebased the remaining patches on my to-upstream branch onto the
> current release of cygport:
> 
> https://repo.or.cz/cygport/rpm-style.git/shortlog/refs/heads/to-upstream
> 
> Note that some of these are required to correctly build and distribute
> Perl and its modules for Cygwin (which is one reason I carry these for
> several years now).

A few comments after looking at:

lib/pkg_info.cygport: implement automatic determination of the 
appropriate perl5_0xy requirement
1. In __list_deps(), this should look at the files list in $@, not at 
files in $D, as that causes it to identify a perl5_0xy dependency for 
all subpackages, irrespective of which package (if any) contains the 
files in the vendor_perl directory.

2. This only identifies the perl5_0xy requirement for packages which own 
files in the vendor_perl directory, not for packages which contain 
executables or shared libraries dynamically linked with cygperl5_xy.dll. 
  That should at least be mentioned in the patch commentary.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: cygport
  2022-03-14 19:06 ` cygport Jon Turney
@ 2022-03-14 20:12   ` Achim Gratz
  2022-03-14 20:50     ` cygport Jon Turney
  2022-03-27 13:22   ` cygport Achim Gratz
  1 sibling, 1 reply; 9+ messages in thread
From: Achim Gratz @ 2022-03-14 20:12 UTC (permalink / raw)
  To: cygwin-apps

Jon Turney writes:
> lib/pkg_info.cygport: implement automatic determination of the
> appropriate perl5_0xy requirement
> 1. In __list_deps(), this should look at the files list in $@, not at
> files in $D, as that causes it to identify a perl5_0xy dependency for 
> all subpackages, irrespective of which package (if any) contains the
> files in the vendor_perl directory.

I guess I didn't have a package with those characteristics.  Will change.

> 2. This only identifies the perl5_0xy requirement for packages which
> own files in the vendor_perl directory, not for packages which contain 
> executables or shared libraries dynamically linked with
> cygperl5_xy.dll.   That should at least be mentioned in the patch
> commentary.

Hmm.  Example package to test this with?


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: cygport
  2022-03-14 20:12   ` cygport Achim Gratz
@ 2022-03-14 20:50     ` Jon Turney
  0 siblings, 0 replies; 9+ messages in thread
From: Jon Turney @ 2022-03-14 20:50 UTC (permalink / raw)
  To: cygwin-apps

On 14/03/2022 20:12, Achim Gratz wrote:
> Jon Turney writes:
>> lib/pkg_info.cygport: implement automatic determination of the
>> appropriate perl5_0xy requirement
>> 1. In __list_deps(), this should look at the files list in $@, not at
>> files in $D, as that causes it to identify a perl5_0xy dependency for
>> all subpackages, irrespective of which package (if any) contains the
>> files in the vendor_perl directory.
> 
> I guess I didn't have a package with those characteristics.  Will change.
> 
>> 2. This only identifies the perl5_0xy requirement for packages which
>> own files in the vendor_perl directory, not for packages which contain
>> executables or shared libraries dynamically linked with
>> cygperl5_xy.dll.   That should at least be mentioned in the patch
>> commentary.
> 
> Hmm.  Example package to test this with?

The list I have (which might not be correct, and probably isn't 
complete) is:

known_packages = [
     'perl-gdal',
     'hexchat-perl',
     'irssi',
     'postgresql-contrib',
     'postgresql-plperl',
     'rxvt-unicode',
     'weechat-perl',
     'znc-perl',
]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: cygport
  2022-03-14 19:06 ` cygport Jon Turney
  2022-03-14 20:12   ` cygport Achim Gratz
@ 2022-03-27 13:22   ` Achim Gratz
  2022-04-10 18:44     ` cygport Jon Turney
  1 sibling, 1 reply; 9+ messages in thread
From: Achim Gratz @ 2022-03-27 13:22 UTC (permalink / raw)
  To: cygwin-apps

Jon Turney writes:
> A few comments after looking at:
>
> lib/pkg_info.cygport: implement automatic determination of the
> appropriate perl5_0xy requirement
> 1. In __list_deps(), this should look at the files list in $@, not at
> files in $D, as that causes it to identify a perl5_0xy dependency for 
> all subpackages, irrespective of which package (if any) contains the
> files in the vendor_perl directory.
>
> 2. This only identifies the perl5_0xy requirement for packages which
> own files in the vendor_perl directory, not for packages which contain 
> executables or shared libraries dynamically linked with
> cygperl5_xy.dll.   That should at least be mentioned in the patch
> commentary.

I've fixed both of these on the to-upstream branch I think.


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: cygport
  2022-03-27 13:22   ` cygport Achim Gratz
@ 2022-04-10 18:44     ` Jon Turney
  2022-04-10 18:57       ` cygport Achim Gratz
  0 siblings, 1 reply; 9+ messages in thread
From: Jon Turney @ 2022-04-10 18:44 UTC (permalink / raw)
  To: cygwin-apps, Achim Gratz

On 27/03/2022 14:22, Achim Gratz wrote:
> Jon Turney writes:
>> A few comments after looking at:
>>
>> lib/pkg_info.cygport: implement automatic determination of the
>> appropriate perl5_0xy requirement
>> 1. In __list_deps(), this should look at the files list in $@, not at
>> files in $D, as that causes it to identify a perl5_0xy dependency for
>> all subpackages, irrespective of which package (if any) contains the
>> files in the vendor_perl directory.
>>
>> 2. This only identifies the perl5_0xy requirement for packages which
>> own files in the vendor_perl directory, not for packages which contain
>> executables or shared libraries dynamically linked with
>> cygperl5_xy.dll.   That should at least be mentioned in the patch
>> commentary.
> 
> I've fixed both of these on the to-upstream branch I think.

Thanks.

I tried testing this on rxvt-unicode.  It correctly adds the perl5_032 
dependency due to linkage

But it also emits a bogus dependency on 'Carp'.

> @@ -412,6 +413,7 @@ __list_deps() {
>                         do
>                                 if [ -f ${d}/${pldep//:://}.pm ]
>                                 then
> +                                       case "${d##*/}" in 5.[0-9][0-9]) plver+="$pldep " ;; esac

Is the mistake thinking pldep here is a pathname, not a module name?

>                                         alldeps+=" "${d}/${pldep//:://}.pm;
>                                         break;
>                                 fi
> @@ -419,6 +421,17 @@ __list_deps() {
>                 done
>         fi
> 
> +       plver=( $( echo "${plver}" | tr ' ' '\n' | sed -e 's/.*\///;s/5/perl5/;s/\./_0/' | sort -ru ) )
> +       if [ "${#plver[@]}" -gt 1 ]
> +       then
> +               warning "More than one targeted Perl version: ${plver[*]},"
> +               warning "using only the latest as dependency: ${plver[0]}."
> +       fi
> +       if [ "${#plver[@]}" -gt 0 ] && [ "${PN}" != "perl_base" ]
> +       then
> +               echo "${plver[0]}"
> +       fi
> +
>         if check_prog php-config
>         then
>                 phpmoddir=$(php-config  --extension-dir)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: cygport
  2022-04-10 18:44     ` cygport Jon Turney
@ 2022-04-10 18:57       ` Achim Gratz
  0 siblings, 0 replies; 9+ messages in thread
From: Achim Gratz @ 2022-04-10 18:57 UTC (permalink / raw)
  To: cygwin-apps

Jon Turney writes:
>> @@ -412,6 +413,7 @@ __list_deps() {
>>                         do
>>                                 if [ -f ${d}/${pldep//:://}.pm ]
>>                                 then
>> +                                       case "${d##*/}" in 5.[0-9][0-9]) plver+="$pldep " ;; esac
>
> Is the mistake thinking pldep here is a pathname, not a module name?

Most likely yes, although I'd have to see what kind of data triggers
this clause.


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

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: cygport
  2022-01-10 18:10 cygport Achim Gratz
  2022-03-14 19:06 ` cygport Jon Turney
@ 2022-09-15 17:45 ` Jon Turney
  2022-09-15 19:34   ` cygport Achim Gratz
  1 sibling, 1 reply; 9+ messages in thread
From: Jon Turney @ 2022-09-15 17:45 UTC (permalink / raw)
  To: Achim Gratz, cygwin-apps

On 10/01/2022 18:10, Achim Gratz wrote:
> 
> I've rebased the remaining patches on my to-upstream branch onto the
> current release of cygport:
> 
> https://repo.or.cz/cygport/rpm-style.git/shortlog/refs/heads/to-upstream
> 
> Note that some of these are required to correctly build and distribute
> Perl and its modules for Cygwin (which is one reason I carry these for
> several years now).

I'm not keen on reviewing patches supplied that way, but ok...

> 5bc72c1 lib/pkg_pkg: allow suppression of spurious package dependencies

As far as I recall, previous discussion of this petered out with "please 
give a concrete example where the dependency autodetection is wrong, and 
explain why it can't be fixed".

If I'm misremembering, please point that out.

The new variable this introduces needs documenting.

> f5764cf lib/pkg_info.cygport: implement automatic determination of the appropriate perl5_0xy requirement

This looks like a fix for the previous commit, mixed in with some rebase 
detritus.

> f1fdfb4 lib/src_prep.cygpart: process various checksum digests

The change to cygpatch to allow .xz and .zst compressed patches needs 
breaking out as a separate change.  That should also improve the 
documentation of PATCH_URI to mention that it handles compressed patch 
files transparently.

This needs a documentation change to mention when prep will verify 
checksums.

It seems that __chksum_verify() ignores the result of running *sum.  Why?

Ideally, a test should be added or extended to exercise this.

> 63e00e5 lib/src_prep.cygpart: determine and deal correctly with another type of checksum

This should be combined with the previous patch?

> 8a325c5 bin/cygport.in: make system-wide defaults overrideable by user defaults and provide ability to change initial MAKEOPTS via CYGPORT_MAKEOPTS

This seems to be two separate changes.

The documentation for cygport.conf should be updated to reflect that 
~/.cygport.conf overrides /etc/cygport.conf.

What it the use case for being able to override MAKEOPTS with a 
environment variable?

CYGPORT_MAKEOPTS needs documenting.

> 74935d6 bin/cygport.in, lib/help.cygpart: implement --jobs/-j N option to specify number of jobs to use

This seems to contain part of the previous change, removing the break 
when looping over config files.

Why do we need both -j and CYGPORT_MAKEOPTS?

> 7777191 allow for different package compression types and implement ZStandard decompression

The change to unpack .tar.zst or .zst sources needs to be separate.

Ideally, a test should be added or extended to exercise that.

If the intention is to set CYGPORT_TAR_EXT and CYGPORT_TAR_CMD in the 
cygport, I don't think they need the CYGPORT_ prefix.

These variables need documenting.

This seems like a weird implementation to me.  Why can't cygport set 
TAR_CMD to invoke tar with the appropriate compression option, depending 
on what TAR_EXT is set to?

> 34502d2 (stromenko/to-upstream) lib/src_install.cygpart: make_etc_defaults, create diff if files aren't matching

This seems kind of useful, but what's the reasoning behind saving a diff 
vs. just saving a backup copy of the previous file?

The documentation for make_etc_defaults should mention this behaviour.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: cygport
  2022-09-15 17:45 ` cygport Jon Turney
@ 2022-09-15 19:34   ` Achim Gratz
  0 siblings, 0 replies; 9+ messages in thread
From: Achim Gratz @ 2022-09-15 19:34 UTC (permalink / raw)
  To: cygwin-apps

Jon Turney writes:
> I'm not keen on reviewing patches supplied that way, but ok...

Suggest an alternative perhaps?

>> 5bc72c1 lib/pkg_pkg: allow suppression of spurious package dependencies
>
> As far as I recall, previous discussion of this petered out with
> "please give a concrete example where the dependency autodetection is
> wrong, and explain why it can't be fixed".
>
> If I'm misremembering, please point that out.

I need to use this in perl.cygport or else perl_base creates a circular
dependency to perl.  Perl has several modules (the official one is
Module::ScanDeps) that try to figure out which deppendencies a Perl
program might have, but even these produce both false positives and
negatives.  The simplistic grep/sed filter that fishes out everything
that looks like a use or require statement is mostly produces  false
positives, since it doesn't consider conditionals.

> The new variable this introduces needs documenting.
>
>> f5764cf lib/pkg_info.cygport: implement automatic determination of the appropriate perl5_0xy requirement
>
> This looks like a fix for the previous commit, mixed in with some
> rebase detritus.

Yeah, I guess the pkg_bin_requires part hopped over the previous patch.

>> f1fdfb4 lib/src_prep.cygpart: process various checksum digests
>
> The change to cygpatch to allow .xz and .zst compressed patches needs
> breaking out as a separate change.  That should also improve the
> documentation of PATCH_URI to mention that it handles compressed patch
> files transparently.

OK.

> This needs a documentation change to mention when prep will verify
> checksums.
>
> It seems that __chksum_verify() ignores the result of running *sum.  Why?

The test is not very robust against hand-produced or otherwise slightly
modified checksum files.  In any case it follows the example of the GPG
signature check in that regard.

> Ideally, a test should be added or extended to exercise this.
>
>> 63e00e5 lib/src_prep.cygpart: determine and deal correctly with another type of checksum
>
> This should be combined with the previous patch?

Hysterical raisins… that was in fact added later since I hadn't
encountered one of those before.  I can merge the two patches no
problem.

>> 8a325c5 bin/cygport.in: make system-wide defaults overrideable by user defaults and provide ability to change initial MAKEOPTS via CYGPORT_MAKEOPTS
>
> This seems to be two separate changes.
>
> The documentation for cygport.conf should be updated to reflect that
> ~/.cygport.conf overrides /etc/cygport.conf.
>
> What it the use case for being able to override MAKEOPTS with a
> environment variable?

I've ran into trouble with Makefiles that are not parallel safe a few
times, so I wanted a way of playing with that without having to modify
the cygport each time.  It's more convenient to specify that on the
command line.

> CYGPORT_MAKEOPTS needs documenting.

OK.

>> 74935d6 bin/cygport.in, lib/help.cygpart: implement --jobs/-j N option to specify number of jobs to use
>
> This seems to contain part of the previous change, removing the break
> when looping over config files.
>
> Why do we need both -j and CYGPORT_MAKEOPTS?

Well strictly we don't need it, ensuring that the approrpiate number of
processors get used is important to me for parallel package builds (the
more packages that can be built in parallel, the less number of cores
each individual job gets assigned).

CYGPORT_MAKEOPTS was introduced and proved useful for debugging much
earlier, so I kept it around even though I don't want to use it for the
purpose of just controlling the number of job slots.

>> 7777191 allow for different package compression types and implement ZStandard decompression
>
> The change to unpack .tar.zst or .zst sources needs to be separate.

OK.

> Ideally, a test should be added or extended to exercise that.
>
> If the intention is to set CYGPORT_TAR_EXT and CYGPORT_TAR_CMD in the
> cygport, I don't think they need the CYGPORT_ prefix.

Since these variables bleed into the environment I'd like to play it safe.

> These variables need documenting.
>
> This seems like a weird implementation to me.  Why can't cygport set
> TAR_CMD to invoke tar with the appropriate compression option,
> depending on what TAR_EXT is set to?

Because not all tar implementations support auto-compression based on
the extension.  In particular, Cygwin's tar did not yet recognise .zst
as a valid extension when this was introduced.  The other reason is that
with auto-compression the compression level can not be modified for
several compression types and it's useful to change some other things
via this facility.  This was briefly discussed when the patch was posted
initially:

https://inbox.sourceware.org/cygwin-apps/87tuzd7vyp.fsf@Rainer.invalid/

>> 34502d2 (stromenko/to-upstream) lib/src_install.cygpart: make_etc_defaults, create diff if files aren't matching
>
> This seems kind of useful, but what's the reasoning behind saving a
> diff vs. just saving a backup copy of the previous file?

You mean like rpm does?  The first thing I always do with .rpmnew or
.rpmsave is to run a diff so I can see which changes to keep and which
to skip.

> The documentation for make_etc_defaults should mention this behaviour.

OK.


This will take a while I think.


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

Samples for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldSamplesExtra

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-09-15 19:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-10 18:10 cygport Achim Gratz
2022-03-14 19:06 ` cygport Jon Turney
2022-03-14 20:12   ` cygport Achim Gratz
2022-03-14 20:50     ` cygport Jon Turney
2022-03-27 13:22   ` cygport Achim Gratz
2022-04-10 18:44     ` cygport Jon Turney
2022-04-10 18:57       ` cygport Achim Gratz
2022-09-15 17:45 ` cygport Jon Turney
2022-09-15 19:34   ` cygport Achim Gratz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).