`guix lint` should not check patch-file-names on inherited sources

  • Open
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • Marius Bakke
Owner
unassigned
Submitted by
Marius Bakke
Severity
minor
M
M
Marius Bakke wrote on 19 Oct 2016 15:51
(address . bug-guix@gnu.org)
87zim05tvp.fsf@duckhunt.i-did-not-set--mail-host-address--so-tickle-me
After patching 'notmuch', `guix lint -c patch-file-names` does not pass
for 'python-notmuch' which inherits the source from 'notmuch'.
L
L
Ludovic Courtès wrote on 19 Oct 2016 21:51
(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 24737@debbugs.gnu.org)
87eg3c5d8b.fsf@gnu.org
Marius Bakke <mbakke@fastmail.com> skribis:

Toggle quote (3 lines)
> After patching 'notmuch', `guix lint -c patch-file-names` does not pass
> for 'python-notmuch' which inherits the source from 'notmuch'.

I agree but that’s not quite possible: the “inheritance” relation (which
is really just a copy of a record) is not known at run time.

So we’d need another trick to guess whether a patch is coming from
elsewhere and should consequently be ignored by ‘lint’.

Thanks,
Ludo’.
M
M
Marius Bakke wrote on 22 Oct 2016 20:53
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 24737@debbugs.gnu.org)
87insk5i7l.fsf@duckhunt.i-did-not-set--mail-host-address--so-tickle-me
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (11 lines)
> Marius Bakke <mbakke@fastmail.com> skribis:
>
>> After patching 'notmuch', `guix lint -c patch-file-names` does not pass
>> for 'python-notmuch' which inherits the source from 'notmuch'.
>
> I agree but that’s not quite possible: the “inheritance” relation (which
> is really just a copy of a record) is not known at run time.
>
> So we’d need another trick to guess whether a patch is coming from
> elsewhere and should consequently be ignored by ‘lint’.

Here is a "RFC" patch that thwarts the warning if the source file name
is different from the package name. Not sure how to properly make it
part of the procedure, so that the checks are actually skipped as well.
From 160132bdc23b34c6331adf00af46af19dd8d737c Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Sat, 22 Oct 2016 19:12:00 +0100
Subject: [PATCH] lint: Skip 'patch-file-names' on inherited sources.

* guix/scripts/lint.scm (check-patch-file-names): Only report when the
source file name matches the package name.
---
guix/scripts/lint.scm | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

Toggle diff (24 lines)
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index d6281ea..ba1bbc3 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -497,10 +497,13 @@ patch could not be found."
(_ #f)) ;must be an <origin> or something like that.
(or (and=> (package-source package) origin-patches)
'()))
- (emit-warning
- package
- (_ "file names of patches should start with the package name")
- 'patch-file-names))))
+ ;; Skip report when the source file name differs (i.e. inherited).
+ (and (string-prefix? (package-name package)
+ (origin-actual-file-name (package-source package)))
+ (emit-warning
+ package
+ (_ "file names of patches should start with the package name")
+ 'patch-file-names)))))
(define (escape-quotes str)
"Replace any quote character in STR by an escaped quote character."
--
2.10.1
M
M
Marius Bakke wrote on 22 Oct 2016 21:07
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 24737@debbugs.gnu.org)
87funo5hjw.fsf@duckhunt.i-did-not-set--mail-host-address--so-tickle-me
Marius Bakke <mbakke@fastmail.com> writes:

Toggle quote (17 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Marius Bakke <mbakke@fastmail.com> skribis:
>>
>>> After patching 'notmuch', `guix lint -c patch-file-names` does not pass
>>> for 'python-notmuch' which inherits the source from 'notmuch'.
>>
>> I agree but that’s not quite possible: the “inheritance” relation (which
>> is really just a copy of a record) is not known at run time.
>>
>> So we’d need another trick to guess whether a patch is coming from
>> elsewhere and should consequently be ignored by ‘lint’.
>
> Here is a "RFC" patch that thwarts the warning if the source file name
> is different from the package name. Not sure how to properly make it
> part of the procedure, so that the checks are actually skipped as well.

I just realized this approach will skip this check completely, if there
are no packages that are named the same as origin (e.g. in the case of
the soon-to-be-added avro, where the source is shared between the
various avro-{c,python,java} etc packages.)

The best approach is probably to check patch-file-names against
(origin-actual-file-name (package-source package)), assuming one can
extract the "base" name of origin-actual-file-name reliably.
L
L
Ludovic Courtès wrote on 24 Oct 2016 22:03
(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 24737@debbugs.gnu.org)
871sz5345z.fsf@gnu.org
Marius Bakke <mbakke@fastmail.com> skribis:

Toggle quote (28 lines)
> Marius Bakke <mbakke@fastmail.com> writes:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Marius Bakke <mbakke@fastmail.com> skribis:
>>>
>>>> After patching 'notmuch', `guix lint -c patch-file-names` does not pass
>>>> for 'python-notmuch' which inherits the source from 'notmuch'.
>>>
>>> I agree but that’s not quite possible: the “inheritance” relation (which
>>> is really just a copy of a record) is not known at run time.
>>>
>>> So we’d need another trick to guess whether a patch is coming from
>>> elsewhere and should consequently be ignored by ‘lint’.
>>
>> Here is a "RFC" patch that thwarts the warning if the source file name
>> is different from the package name. Not sure how to properly make it
>> part of the procedure, so that the checks are actually skipped as well.
>
> I just realized this approach will skip this check completely, if there
> are no packages that are named the same as origin (e.g. in the case of
> the soon-to-be-added avro, where the source is shared between the
> various avro-{c,python,java} etc packages.)
>
> The best approach is probably to check patch-file-names against
> (origin-actual-file-name (package-source package)), assuming one can
> extract the "base" name of origin-actual-file-name reliably.

(‘origin-actual-file-name’ already returns a basename.)

Could you check whether the patch your proposed works well for some of
the annoying cases we currently have, and also adds those as test cases
in ‘tests/lint.scm’? (See the manual on how to run the tests (info
"(guix) Running the Test Suite").)

If that works well enough, we should go for it.

The only 100% reliable way to address this, I think, would be to build a
patch to package mapping, and then make sure that for each patch, at
least one of the corresponding packages has a matching name. The
problem is that ‘lint’ is currently designed to work one package at a
time.

Thanks!

Ludo’.
L
L
Ludovic Courtès wrote on 2 Nov 2016 23:18
control message for bug #24737
(address . control@debbugs.gnu.org)
87h97ple5j.fsf@gnu.org
severity 24737 minor
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 24737@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 24737
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch