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

OpenSubmitted by Marius Bakke.
Details
2 participants
  • Ludovic Courtès
  • Marius Bakke
Owner
unassigned
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 passfor '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 (whichis 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 fromelsewhere 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 nameis different from the package name. Not sure how to properly make itpart of the procedure, so that the checks are actually skipped as well.
From 160132bdc23b34c6331adf00af46af19dd8d737c Mon Sep 17 00:00:00 2001From: Marius Bakke <mbakke@fastmail.com>Date: Sat, 22 Oct 2016 19:12:00 +0100Subject: [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.scmindex 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 thereare no packages that are named the same as origin (e.g. in the case ofthe soon-to-be-added avro, where the source is shared between thevarious 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 canextract 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 ofthe annoying cases we currently have, and also adds those as test casesin ‘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 apatch to package mapping, and then make sure that for each patch, atleast one of the corresponding packages has a matching name. Theproblem is that ‘lint’ is currently designed to work one package at atime.
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
?