diff mbox series

[bug#51985] lint: Adjust patch file length check.

Message ID 875ysnsvnt.fsf@yucca
State Accepted
Headers show
Series [bug#51985] lint: Adjust patch file length check. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Vagrant Cascadian Nov. 19, 2021, 9:05 p.m. UTC 
The current guix lint check is a bit overly conservative, and reports
several results which do not in practice actually cause issues.

This patch proposes to reduce the size by two characters (leaving only
two patches on guix master that need to be adjusted), uses a version
string more like what actually might be included in a tarball built
using "make dist", and adds a comment describing what the arbitrary
string actually is supposed to represent.

This should still even leave a little wiggle-room when guix hits version
100+ and/or 1000000+ commits, by which time hopefully guix has switched
to a tarball format that doesn't have such a short arbitrary file length
limit!


live well,
  vagrant

Comments

Ludovic Courtès Nov. 22, 2021, 11:22 a.m. UTC | #1 
Hi,

Vagrant Cascadian <vagrant@debian.org> skribis:

> From 6ad2050a8bbc308a328d30d4f66cb229d868b79d Mon Sep 17 00:00:00 2001
> From: Vagrant Cascadian <vagrant@debian.org>
> Date: Fri, 19 Nov 2021 12:14:19 -0800
> Subject: [PATCH] lint: Adjust patch file length check.
>
> * guix/lint.scm (check-patch-file-names): Adjust margin used to check for
>   patch file lengths.

Make sure “make check TESTS=tests/lint.scm” is still happy, but
otherwise LGTM.

Thanks!

Ludo’.
Vagrant Cascadian Nov. 24, 2021, 9:25 p.m. UTC | #2 
On 2021-11-22, Ludovic Courtès wrote:
> Vagrant Cascadian <vagrant@debian.org> skribis:
>> From 6ad2050a8bbc308a328d30d4f66cb229d868b79d Mon Sep 17 00:00:00 2001
>> From: Vagrant Cascadian <vagrant@debian.org>
>> Date: Fri, 19 Nov 2021 12:14:19 -0800
>> Subject: [PATCH] lint: Adjust patch file length check.
>>
>> * guix/lint.scm (check-patch-file-names): Adjust margin used to check for
>>   patch file lengths.
>
> Make sure “make check TESTS=tests/lint.scm” is still happy, but
> otherwise LGTM.

With:

  commit bdf5c16ac052af2ca9d5c3acc4acbc08fd9fdbea
  Author: Ludovic Courtès <ludo@gnu.org>
  Date:   Tue Nov 23 09:06:49 2021 +0100

    maint: "make dist" builds tarballs in 'ustar' format.

It seems like this actually needs even further updates, as that should
allow for a much longer file length in general (although a little
difficult to figure out the exact file length allowed).

And then the corresponding test suite will need changes as well...

live well,
  vagrant
Ludovic Courtès Nov. 25, 2021, 1:08 p.m. UTC | #3 
Hi,

Vagrant Cascadian <vagrant@debian.org> skribis:

> With:
>
>   commit bdf5c16ac052af2ca9d5c3acc4acbc08fd9fdbea
>   Author: Ludovic Courtès <ludo@gnu.org>
>   Date:   Tue Nov 23 09:06:49 2021 +0100
>
>     maint: "make dist" builds tarballs in 'ustar' format.
>
> It seems like this actually needs even further updates, as that should
> allow for a much longer file length in general (although a little
> difficult to figure out the exact file length allowed).
>
> And then the corresponding test suite will need changes as well...

I think independently of the switch to ustar, it’s a good idea for ‘guix
lint’ to warn about long patch file names, but to warn a bit less
frequently than today.

In that spirit, your patch is still relevant and worth applying IMO.

Thoughts?

Ludo’.
diff mbox series

Patch

From 6ad2050a8bbc308a328d30d4f66cb229d868b79d Mon Sep 17 00:00:00 2001
From: Vagrant Cascadian <vagrant@debian.org>
Date: Fri, 19 Nov 2021 12:14:19 -0800
Subject: [PATCH] lint: Adjust patch file length check.

* guix/lint.scm (check-patch-file-names): Adjust margin used to check for
  patch file lengths.
---
 guix/lint.scm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/guix/lint.scm b/guix/lint.scm
index ac2e7b3841..39b4a2ae85 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -957,7 +957,10 @@  patch could not be found."
 
      ;; Check whether we're reaching tar's maximum file name length.
      (let ((prefix (string-length (%distro-directory)))
-           (margin (string-length "guix-2.0.0rc3-10000-1234567890/"))
+           ;; Margin approximating the largest path that "make dist" might
+           ;; create, with a release candidate version, 123456 commits, and
+           ;; git commit hash abcde0.
+           (margin (string-length "guix-12.0.0rc3-123456-abcde0/"))
            (max    99))
        (filter-map (match-lambda
                      ((? string? patch)
-- 
2.30.2