Re: [PATCH] gnu: spice: Fix usbredir for 32 bit platforms.

[Mark H Weaver]

David Craven <address@hidden> writes:

>> Phase procedures are supposed to return a boolean indicating whether
>> they succeeded, but 'system*' returns a number: a result code.  In
>> scheme, all numbers are considered true.  Also, you might as well use
>> 'lambda' here instead of 'lambda*', so it should look like this:
>
>>           (lambda _
>>             (zero? (system* "sh" "autogen.sh")))
>
> The reason was that autogen.sh performs some check at the end that fails.
> I'm running autoreconf directly now, so that the zero? doesn't cause the
> phase to fail.

Sounds good, thanks.

>> This is no longer version "0.7.1", so the version number needs to be
>> updated accordingly.  Please see section 7.6.3 (Version Numbers) in the
>> manual for our conventions for version numbers of VCS snapshots, and the
>> recommended code to generate those version numbers.
>
> This could have also probably survived until the package gets updated, but
> I fixed it.

Thank you.  I think it's important for the 'version' field to be
accurate.  If you could push this fix at your earliest convenience, I'd
be grateful.  (I don't see it in the git repo yet)

>> IT might have been better to just use a simple patch to fix the format
>> strings than to use a VCS snapshot that might introduce more bugs, but
>> I guess we can see how it goes.
>
> I considered this option also, but in at least one instance I was asked to
> use substitute* instead of a patch, so I thought that patches are considered
> a last resort.

They are certainly not a last resort in general.  The preferred method
of making changes prior to the build depends on the specific details.

However, one important consideration is that patches and snippets change
what is considered by Guix to be the "source" of the package as returned
by 'package-source' at the Scheme level and "guix build --source" as the
command line.

With this in mind, I'd say that for bug fixes, patches are generally the
preferred method.

> I also checked the history before selecting HEAD as the commit,
> it looks like there where only a couple of bugfixes, but not much activity
> otherwise. So hopefully this does not introduce any new bugs.

Oh, okay, that makes sense.  Thanks for working on it.

      Mark