public inbox for
 help / color / mirror / Atom feed
From: Achim Gratz <>
Subject: Re: cygport
Date: Thu, 15 Sep 2022 21:34:18 +0200	[thread overview]
Message-ID: <87o7vg4ekl.fsf@Rainer.invalid> (raw)
In-Reply-To: <> (Jon Turney's message of "Thu, 15 Sep 2022 18:45:24 +0100")

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.


> 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

>> 8a325c5 bin/ 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.


>> 74935d6 bin/, 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.


> 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

>> 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.


This will take a while I think.

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

Samples for the Waldorf Blofeld:

      reply	other threads:[~2022-09-15 19:34 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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   ` Achim Gratz [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87o7vg4ekl.fsf@Rainer.invalid \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).