GNU bug report logs - #66870
[PATCH 0/6] Add yabridge.

Previous Next

Full log

Message #37 received at 66870 <at> debbugs.gnu.org (full text, mbox):

From: sughosha <sughosha <at> disroot.org>
To: Gabriel Wicki <gabriel <at> erlikon.ch>
Cc: 66870 <at> debbugs.gnu.org
Subject: Re: Review
Date: Thu, 30 Nov 2023 09:15:15 +0100

Hi,

Thanks for the review.

> So i've de-frizzled the patches by renaming #66870 to "[PATCH 0/6] Add
> yabridge."

Thank you.

> [PATCH 1/6] looks good to me! Have you checked the packages depending
> on asio still build and work?

I checked only musikcube, jami and widelands and they worked fine. 
Others
must work as well.

> [PATCH 3/6] Looks goot, although I am a bit confused WRT the comment 
> for
> the (recursive #t) clause in the (source ..) field (indicating we need
> to recursively copy the repo for the tests to complete) and argument
> (#:tests? #f) indicating that the tests wouldn't be run.

Removed "(recursive #t)".

> - I'm not sure whether your (let ((arch))) clause isn't an abuse; you
> test whether (%current-system) is either x86_64-linux or aarch64-linux
> and set it to x86_64-unix and i386-unix otherwise. Does this actually
> work for aarch64-linux systems? You could make use of the
> target-64-bit? function in (guix utils) and only allow the package to
> build on x86 systems. Also, you inherit wine64's supported-systems
> field - which only lists x86_64-linux. Building for i368 seems to be
> unsupported by our wine packages (or wine in general?)

Removed the substitute as it is no more needed.

> - I'm a bit confused about your patch series adding clap 1.1.9 -
> although yabridge seems to need 1.1.7. Why not add clap <at> 1.1.7 with this
> series and with the next upgrade of yabridge also update clap?

Moved clap <at> 1.1.7 to a seperate definition.

> - Similar with input vst3sdk: Why not add it as a separate package?
> If I interpret the situation correctly, using default definitions of
> these inputs could also let you simplify your native-inputs list to the
> new style.

This vst3sdk is not the original one. It is found in a different
repository than the original one and is suppose to clone as subproject. 
So
it does not make sense to package it in a seperate definition.

> $(guix lint) on the patched packages gave the following output:
> 
> ~/src/guix/gnu/packages/networking.scm:3431:15: asio <at> 1.28.0: permanent
> redirect from https://think-async.com/Asio to
> https://think-async.com/Asio/
> fetching CVE database for 2023...
> ~/src/guix/gnu/packages/networking.scm:3417:5: asio <at> 1.28.0:
> source not archived on Swith "(let ((arch)))"oftware Heritage and 
> missing from the Disarchive
> database
> ~/src/guix/gnu/packages/cpp.scm:2311:13: function2 <at> 4.2.3:
> can be upgraded to 4.2.4
> ~/src/guix/gnu/packages/audio.scm:2724:13: clap <at> 1.1.9: can
> be upgraded to 1.1.10
> ~/src/guix/gnu/packages/audio.scm:6002:0: yabridge <at> 5.0.5:
> line 6002 is way too long (95 characters)
> ~/src/guix/gnu/packages/audio.scm:6013:0: yabridge <at> 5.0.5:
> line 6013 is way too long (100 characters)

Fixed and updated these packages.

I will submit the v2 patches with the above changes.

Regards,
Sughosha

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.