From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mr6.vodafonemail.de (mr6.vodafonemail.de [145.253.228.166]) by sourceware.org (Postfix) with ESMTPS id 61A6E3858C62 for ; Thu, 15 Sep 2022 19:34:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 61A6E3858C62 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=nexgo.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nexgo.de Received: from smtp.vodafone.de (unknown [10.0.0.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by mr6.vodafonemail.de (Postfix) with ESMTPS id 4MT6pV5hn3z1y4F for ; Thu, 15 Sep 2022 19:34:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nexgo.de; s=vfde-smtpout-mb-15sep; t=1663270466; bh=DLqS9i4QR5Eac5Pq8B3l8ChOUhreKfT5BYMe/UvuVcM=; h=From:To:Subject:References:Date:In-Reply-To:Message-ID:User-Agent: Content-Type:From; b=qos8puqDlidCpUcDeaeL3uLLtoJtg/JTx6VPi4QKd7+lWPbFyXvvip5cK9TI26aEO HKHdYuzrE0ZosbU1CL0Y9lGZBSdGD7Fc4IDCJEXjiobJ8kkOPywaemuDwZf7AP/MLw +FBpazv2pzDZ9gCN4DSFfFuJBX6KGU/yHe/IK3NQ= Received: from Gertrud (p57b9d3f5.dip0.t-ipconnect.de [87.185.211.245]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (prime256v1) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp.vodafone.de (Postfix) with ESMTPSA id 4MT6pT5FTVzHnHW for ; Thu, 15 Sep 2022 19:34:22 +0000 (UTC) From: Achim Gratz To: cygwin-apps@cygwin.com Subject: Re: cygport References: <87tueb8nry.fsf@Rainer.invalid> <45dd81c8-16fd-d34e-15ab-855727cbbc07@dronecode.org.uk> Date: Thu, 15 Sep 2022 21:34:18 +0200 In-Reply-To: <45dd81c8-16fd-d34e-15ab-855727cbbc07@dronecode.org.uk> (Jon Turney's message of "Thu, 15 Sep 2022 18:45:24 +0100") Message-ID: <87o7vg4ekl.fsf@Rainer.invalid> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-purgate-type: clean X-purgate: clean X-purgate-size: 5645 X-purgate-ID: 155817::1663270465-DAFFC403-F092C264/0/0 X-Spam-Status: No, score=-3031.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 a= ppropriate 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=E2=80=A6 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 d= efaults 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 t= o 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 ZSta= ndard 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_defaul= ts, 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. --=20 +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Samples for the Waldorf Blofeld: http://Synth.Stromeko.net/Downloads.html#BlofeldSamplesExtra