bug#24737: `guix lint` should not check patch-file-names on inherited sources

bug#24737: `guix lint` should not check patch-file-names on inherited sources

[Marius Bakke]

After patching 'notmuch', `guix lint -c patch-file-names` does not pass
for 'python-notmuch' which inherits the source from 'notmuch'.

bug#24737: `guix lint` should not check patch-file-names on inherited sources

[Ludovic Courtès]

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’.

Thanks,
Ludo’.

bug#24737: `guix lint` should not check patch-file-names on inherited sources

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 742 bytes --]

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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lint-Skip-patch-file-names-on-inherited-sources.patch --]
[-- Type: text/x-patch, Size: 1422 bytes --]

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(-)

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

bug#24737: `guix lint` should not check patch-file-names on inherited sources

[Marius Bakke]

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.

bug#24737: `guix lint` should not check patch-file-names on inherited sources

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> 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’.