unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / Atom feed
* default tar format for "make dist" and patch file length
@ 2021-11-16  0:34 Vagrant Cascadian
  2021-11-17 11:32 ` Ludovic Courtès
  0 siblings, 1 reply; 11+ messages in thread
From: Vagrant Cascadian @ 2021-11-16  0:34 UTC (permalink / raw)
  To: guix-devel

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

When running "make dist", guix apparently uses a very old tar format
(presumably for maximal compatibility with tar implementations), which
has a hard limit of 99 characters.

For this reason, there is a guix lint check for filename lengths, but
there are quite a few lint warnings in guix master and
core-updates-frozen.

On guix master db5907138cbf9139e885fa4b3860d604aff0be9c, there appear to
be 14 such patches:

$ guix lint --checkers=patch-file-names | grep 'too long' | nl

     1  gnu/packages/admin.scm:2669:2: debops@1.1.0:
     debops-constants-for-external-program-names.patch: file name is too
     long
     2  gnu/packages/astronomy.scm:865:2: xplanet@1.3.1:
     xplanet-1.3.1-libdisplay_DisplayOutput.cpp.patch: file name is too
     long
     3  gnu/packages/astronomy.scm:865:2: xplanet@1.3.1:
     xplanet-1.3.1-xpUtil-Add2017LeapSecond.cpp.patch: file name is too
     long
     4  gnu/packages/geo.scm:312:2: libgeotiff@1.5.1:
     libgeotiff-adapt-test-script-for-proj-6.2.patch: file name is too
     long
     5  gnu/packages/glib.scm:478:2: gobject-introspection@1.62.0:
     gobject-introspection-absolute-shlib-path.patch: file name is too
     long
     6  gnu/packages/java.scm:9032:2:
     java-tunnelvisionlabs-antlr4-runtime@4.7.4:
     java-tunnelvisionlabs-antlr-code-too-large.patch: file
 name is too long
     7  gnu/packages/java.scm:8926:2:
     java-tunnelvisionlabs-antlr4-runtime-annotations@4.7.4:
     java-tunnelvisionlabs-antlr-code-too-large
.patch: file name is too long
     8  gnu/packages/java.scm:13120:2: java-apache-ivy@2.4.0:
     java-apache-ivy-port-to-latest-bouncycastle.patch: file name is too
     long
     9  gnu/packages/java.scm:9040:2:
     java-tunnelvisionlabs-antlr4@4.7.4:
     java-tunnelvisionlabs-antlr-code-too-large.patch: file name is
 too long
    10  gnu/packages/kde-frameworks.scm:3419:2: plasma-framework@5.70.1:
     plasma-framework-fix-KF5PlasmaMacros.cmake.patch: file name is
too long
    11  gnu/packages/llvm.scm:98:2: clang-runtime@3.9.1:
     clang-runtime-3.9-libsanitizer-mode-field.patch: file name is too
     long
    12  gnu/packages/llvm.scm:852:4: clang-runtime@3.5.2:
     clang-runtime-3.5-libsanitizer-mode-field.patch: file name is too
     long
    13  gnu/packages/llvm.scm:98:2: clang-runtime@3.7.1:
     clang-runtime-3.8-libsanitizer-mode-field.patch: file name is too
     long
    14  gnu/packages/llvm.scm:98:2: clang-runtime@3.8.1:
     clang-runtime-3.8-libsanitizer-mode-field.patch: file name is too long

Looks like a similar number of patches on core-updates-frozen, too,
probably mostly the same ones.

Luckily, the lint check is conservative and actually gives a bit of
wiggle-room, and none of these appear to break "make dist" from
generating a tarball at the moment, but there was one in
core-updates-frozen recently that did break "make dist" (fixed in
6cdf4e5bf230fdbe17e592c2ec74fb34dba70eb5).


Ideally, "guix lint" would be run and issues fixed before applying
patches ... !

Is it worth adding an inexpensive check to etc/git/pre-push that also
checks for file-length and fails to push due to this issue potentially
breaking "make dist"?


A different angle might be to actually use a different tar format:

  https://www.gnu.org/software/tar/manual/html_section/Formats.html

I would guess "make dist" is using the tar "v7" format, based on the 99
character length limit for files. Most of the other formats have no file
length limit or a longer limit.


Well, thanks for following my rambling this far!


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: default tar format for "make dist" and patch file length
  2021-11-16  0:34 default tar format for "make dist" and patch file length Vagrant Cascadian
@ 2021-11-17 11:32 ` Ludovic Courtès
  2021-11-17 22:39   ` Vagrant Cascadian
  0 siblings, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2021-11-17 11:32 UTC (permalink / raw)
  To: Vagrant Cascadian; +Cc: guix-devel

Vagrant Cascadian <vagrant@debian.org> skribis:

> Ideally, "guix lint" would be run and issues fixed before applying
> patches ... !

On the bright side, that there’s just a dozen of issues on 20K packages
suggests it’s usually run.  :-)

I think we’re used to running it for new packages but not when modifying
an existing package, which is probably when issues like that are
introduced.

> Is it worth adding an inexpensive check to etc/git/pre-push that also
> checks for file-length and fails to push due to this issue potentially
> breaking "make dist"?

Could be.

> A different angle might be to actually use a different tar format:
>
>   https://www.gnu.org/software/tar/manual/html_section/Formats.html
>
> I would guess "make dist" is using the tar "v7" format, based on the 99
> character length limit for files. Most of the other formats have no file
> length limit or a longer limit.

Yes, we could also do that.

Thanks,
Ludo’.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: default tar format for "make dist" and patch file length
  2021-11-17 11:32 ` Ludovic Courtès
@ 2021-11-17 22:39   ` Vagrant Cascadian
  2021-11-17 23:49     ` Vagrant Cascadian
  2021-11-22  2:03     ` Maxim Cournoyer
  0 siblings, 2 replies; 11+ messages in thread
From: Vagrant Cascadian @ 2021-11-17 22:39 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

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

On 2021-11-17, Ludovic Courtès wrote:
> Vagrant Cascadian <vagrant@debian.org> skribis:
>
>> Ideally, "guix lint" would be run and issues fixed before applying
>> patches ... !
>
> On the bright side, that there’s just a dozen of issues on 20K packages
> suggests it’s usually run.  :-)
>
> I think we’re used to running it for new packages but not when modifying
> an existing package, which is probably when issues like that are
> introduced.

Sounds plausible.

My guess is this is triggered from folks using "git format-patch" and
dumping the files into gnu/packages/patches, which probably has a
default length that is a little too long in this case.

Another option that would help a little would be to drop the .patch
suffix, it's kind of redundant to have gnu/packages/patches/*.patch

>> Is it worth adding an inexpensive check to etc/git/pre-push that also
>> checks for file-length and fails to push due to this issue potentially
>> breaking "make dist"?
>
> Could be.

This basically mimics the check that guix-lint does:

for p in $(find gnu/packages/patches -type f ) ; do
  if [ "$(echo guix-2.0.0rc3-10000-1234567890/${p} | wc -c)" -ge "99" ]
  then
    echo $p
    exit 1
  fi
done

Would something like that be cheap enough to consider adding to
etc/git/pre-push? Are "find" and "wc" reasonable dependencies to assume
they are available?

Obviously, have to wait until they are all fixed, some of which probably
require going through core-updates... or start with a more conservative
but still useful length-check.


>> A different angle might be to actually use a different tar format:
>>
>>   https://www.gnu.org/software/tar/manual/html_section/Formats.html
>>
>> I would guess "make dist" is using the tar "v7" format, based on the 99
>> character length limit for files. Most of the other formats have no file
>> length limit or a longer limit.
>
> Yes, we could also do that.

Struggling to figure out how to do that; seems automake is very inclined
to use the old format... anyone with sufficient auto* skills to try and
upgrade the "make dist" to pass one of the newer --format= arguments to
tar?


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: default tar format for "make dist" and patch file length
  2021-11-17 22:39   ` Vagrant Cascadian
@ 2021-11-17 23:49     ` Vagrant Cascadian
  2021-11-19 14:54       ` Ludovic Courtès
  2021-11-22  2:03     ` Maxim Cournoyer
  1 sibling, 1 reply; 11+ messages in thread
From: Vagrant Cascadian @ 2021-11-17 23:49 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

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

On 2021-11-17, Vagrant Cascadian wrote:
> On 2021-11-17, Ludovic Courtès wrote:
>> Vagrant Cascadian <vagrant@debian.org> skribis:
>>
>>> Ideally, "guix lint" would be run and issues fixed before applying
>>> patches ... !
...
>>> Is it worth adding an inexpensive check to etc/git/pre-push that also
>>> checks for file-length and fails to push due to this issue potentially
>>> breaking "make dist"?
...
> This basically mimics the check that guix-lint does:
>
> for p in $(find gnu/packages/patches -type f ) ; do
>   if [ "$(echo guix-2.0.0rc3-10000-1234567890/${p} | wc -c)" -ge "99" ]
>   then
>     echo $p
>     exit 1
>   fi
> done
>
> Would something like that be cheap enough to consider adding to
> etc/git/pre-push? Are "find" and "wc" reasonable dependencies to assume
> they are available?
>
> Obviously, have to wait until they are all fixed, some of which probably
> require going through core-updates... or start with a more conservative
> but still useful length-check.

Another strategy would be to reduce the overly cautious lint check:

diff --git a/guix/lint.scm b/guix/lint.scm
index ac2e7b3841..e795c466b1 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -957,7 +957,7 @@ (define (starts-with-package-name? file-name)

      ;; 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 (string-length "guix-2.0.0rc3-10000-12345678/"))
            (max    99))
        (filter-map (match-lambda
                      ((? string? patch)


That leaves only two packages on master in violation of the lint check,
and those are both updatable directly on master, and "make dist" still
works with those two packages as they are, in my experience.

I think that's fairly safe to do, actually, as "make dist" on
core-updates-frozen currently produces a tarball prefixed with:

  guix-1.3.0.10380-fe257/

If guix keeps bumping it's version into the double-digits, an
rc-version, and it surpasses 99999 commits, and an extra character for
the git commit hash, this still leaves considerable wiggle-room:

  guix-10.0.0rc0-123456-abcde0/
vs.
  guix-2.0.0rc3-10000-12345678/

So, I guess I'm leaning towards making the guix lint check a little more
lenient.

Thoughts?


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: default tar format for "make dist" and patch file length
  2021-11-17 23:49     ` Vagrant Cascadian
@ 2021-11-19 14:54       ` Ludovic Courtès
  2021-11-20  4:39         ` Philip McGrath
  0 siblings, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2021-11-19 14:54 UTC (permalink / raw)
  To: Vagrant Cascadian; +Cc: guix-devel

Hello,

Vagrant Cascadian <vagrant@debian.org> skribis:

> Another strategy would be to reduce the overly cautious lint check:
>
> diff --git a/guix/lint.scm b/guix/lint.scm
> index ac2e7b3841..e795c466b1 100644
> --- a/guix/lint.scm
> +++ b/guix/lint.scm
> @@ -957,7 +957,7 @@ (define (starts-with-package-name? file-name)
>
>       ;; 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 (string-length "guix-2.0.0rc3-10000-12345678/"))
>             (max    99))
>         (filter-map (match-lambda
>                       ((? string? patch)
>
>
> That leaves only two packages on master in violation of the lint check,
> and those are both updatable directly on master, and "make dist" still
> works with those two packages as they are, in my experience.
>
> I think that's fairly safe to do, actually, as "make dist" on
> core-updates-frozen currently produces a tarball prefixed with:
>
>   guix-1.3.0.10380-fe257/
>
> If guix keeps bumping it's version into the double-digits, an
> rc-version, and it surpasses 99999 commits, and an extra character for
> the git commit hash, this still leaves considerable wiggle-room:
>
>   guix-10.0.0rc0-123456-abcde0/
> vs.
>   guix-2.0.0rc3-10000-12345678/
>
> So, I guess I'm leaning towards making the guix lint check a little more
> lenient.
>
> Thoughts?

That sounds even better, I’m all for it (changing (guix lint) + fixing
the two remaining issues)!

Ludo’.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: default tar format for "make dist" and patch file length
  2021-11-19 14:54       ` Ludovic Courtès
@ 2021-11-20  4:39         ` Philip McGrath
  2021-11-20  5:21           ` Vagrant Cascadian
  0 siblings, 1 reply; 11+ messages in thread
From: Philip McGrath @ 2021-11-20  4:39 UTC (permalink / raw)
  To: Ludovic Courtès, Vagrant Cascadian; +Cc: guix-devel



On 11/19/21 09:54, Ludovic Courtès wrote:
> Vagrant Cascadian <vagrant@debian.org> skribis:
>> So, I guess I'm leaning towards making the guix lint check a little more
>> lenient.
>>
>> Thoughts?
> 
> That sounds even better, I’m all for it (changing (guix lint) + fixing
> the two remaining issues)!

It might also help to change the warning given by the check.

When a program called "lint" tells me that something is too long, I 
understand that to mean that what I've done is generally considered bad 
style, but there might be a very good reason to do it in some specific 
case. For example, I might exceed a line length guideline to avoid 
inserting linebreaks into a URL.

If instead `guix lint` is telling us about a hard limit that will break 
things, I think it should say so clearly.

-Philip


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: default tar format for "make dist" and patch file length
  2021-11-20  4:39         ` Philip McGrath
@ 2021-11-20  5:21           ` Vagrant Cascadian
  2021-11-24 21:27             ` Vagrant Cascadian
  0 siblings, 1 reply; 11+ messages in thread
From: Vagrant Cascadian @ 2021-11-20  5:21 UTC (permalink / raw)
  To: Philip McGrath, Ludovic Courtès; +Cc: guix-devel, 51775

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

On 2021-11-19, Philip McGrath wrote:
> On 11/19/21 09:54, Ludovic Courtès wrote:
>> Vagrant Cascadian <vagrant@debian.org> skribis:
>>> So, I guess I'm leaning towards making the guix lint check a little more
>>> lenient.
>>>
>>> Thoughts?
>> 
>> That sounds even better, I’m all for it (changing (guix lint) + fixing
>> the two remaining issues)!

Submitted the guix lint change as https://issues.guix.gnu.org/51775


> It might also help to change the warning given by the check.
>
> When a program called "lint" tells me that something is too long, I 
> understand that to mean that what I've done is generally considered bad 
> style, but there might be a very good reason to do it in some specific 
> case. For example, I might exceed a line length guideline to avoid 
> inserting linebreaks into a URL.

That's a good point!


> If instead `guix lint` is telling us about a hard limit that will break 
> things, I think it should say so clearly.

Not sure how to convey succinctly, but here's an attempt at a patch
(which ironically also probably makes the line a bit too long in the
code):

diff --git a/guix/lint.scm b/guix/lint.scm
index ac2e7b3841..6464fb751a 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -968,7 +968,7 @@ (define (starts-with-package-name? file-name)
                              max)
                           (make-warning
                            package
-                           (G_ "~a: file name is too long")
+                           (G_ "~a: file name is too long and may break release tarball generation")
                            (list (basename patch))
                            #:field 'patch-file-names)
                           #f))
diff --git a/tests/lint.scm b/tests/lint.scm
index 9a91dd5426..d4c3d62aaf 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -509,7 +509,7 @@ (define hsab (string-append (assoc-ref inputs
"hsab")
 (test-equal "patches: file name too long"
   (string-append "x-"
                  (make-string 100 #\a)
-                 ".patch: file name is too long")
+                 ".patch: file name is too long and may break release tarball generation")
   (single-lint-warning-message
    (let ((pkg (dummy-package
                "x"


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: default tar format for "make dist" and patch file length
  2021-11-17 22:39   ` Vagrant Cascadian
  2021-11-17 23:49     ` Vagrant Cascadian
@ 2021-11-22  2:03     ` Maxim Cournoyer
  2021-11-22 11:31       ` Ludovic Courtès
  1 sibling, 1 reply; 11+ messages in thread
From: Maxim Cournoyer @ 2021-11-22  2:03 UTC (permalink / raw)
  To: Vagrant Cascadian; +Cc: guix-devel

Hello,

[...]

>>> A different angle might be to actually use a different tar format:
>>>
>>>   https://www.gnu.org/software/tar/manual/html_section/Formats.html
>>>
>>> I would guess "make dist" is using the tar "v7" format, based on the 99
>>> character length limit for files. Most of the other formats have no file
>>> length limit or a longer limit.
>>
>> Yes, we could also do that.
>
> Struggling to figure out how to do that; seems automake is very inclined
> to use the old format... anyone with sufficient auto* skills to try and
> upgrade the "make dist" to pass one of the newer --format= arguments to
> tar?

Reading the Automake manual (info (automake) List of Automake options) I
stumbled on this:

    ‘filename-length-max=99’
         Abort if file names longer than 99 characters are found during
         ‘make dist’.  Such long file names are generally considered not to
         be portable in tarballs.  See the ‘tar-v7’ and ‘tar-ustar’ options
         below.  This option should be used in the top-level ‘Makefile.am’
         or as an argument of ‘AM_INIT_AUTOMAKE’ in ‘configure.ac’; it will
         be ignored otherwise.  It will also be ignored in sub-packages of
         nested packages (*note Subpackages::).

This makes me think that Automake is simply configured out of the box to
keep the file names as portable as possible (it doesn't mean it uses
tar-v7 itself, IIUC, though I haven't checked).

Seems a good thing to be as portable as can be, especially since 200
chars patch file names wouldn't look good in the sources anyway ;-).

Thanks,

Maxim


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: default tar format for "make dist" and patch file length
  2021-11-22  2:03     ` Maxim Cournoyer
@ 2021-11-22 11:31       ` Ludovic Courtès
  2021-11-22 20:14         ` Maxim Cournoyer
  0 siblings, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2021-11-22 11:31 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: Vagrant Cascadian, guix-devel

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

Hi!

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Reading the Automake manual (info (automake) List of Automake options) I
> stumbled on this:
>
>     ‘filename-length-max=99’
>          Abort if file names longer than 99 characters are found during
>          ‘make dist’.  Such long file names are generally considered not to
>          be portable in tarballs.  See the ‘tar-v7’ and ‘tar-ustar’ options
>          below.  This option should be used in the top-level ‘Makefile.am’
>          or as an argument of ‘AM_INIT_AUTOMAKE’ in ‘configure.ac’; it will
>          be ignored otherwise.  It will also be ignored in sub-packages of
>          nested packages (*note Subpackages::).
>
> This makes me think that Automake is simply configured out of the box to
> keep the file names as portable as possible (it doesn't mean it uses
> tar-v7 itself, IIUC, though I haven't checked).

Oh, looks like we could add the ‘tar-ustar’ option and be done with it
(info "(automake) List of Automake options"):

     ‘tar-ustar’ selects the ustar format defined by POSIX 1003.1-1988.
     This format is old enough to be portable: As of 2018, it is
     supported by the native ‘tar’ command on GNU, FreeBSD, NetBSD,
     OpenBSD, AIX, HP-UX, and Solaris, at least.  It fully supports
     empty directories.  It can store file names with up to 256
     characters, provided that the file name can be split at directory
     separator in two parts, first of them being at most 155 bytes long.
     So, in most cases the maximum file name length will be shorter than
     256 characters.

Any objections against the patch below?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 448 bytes --]

diff --git a/configure.ac b/configure.ac
index 84592f6041..341cff8fbd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -8,7 +8,7 @@ AC_INIT([GNU Guix],
   [https://www.gnu.org/software/guix/])
 AC_CONFIG_AUX_DIR([build-aux])
 
-AM_INIT_AUTOMAKE([1.14 gnu silent-rules subdir-objects \
+AM_INIT_AUTOMAKE([1.14 gnu tar-ustar silent-rules subdir-objects \
  color-tests parallel-tests -Woverride -Wno-portability])
 
 # Enable silent rules by default.

[-- Attachment #3: Type: text/plain, Size: 1735 bytes --]


I tried it and there’s no tar warning and the file looks just fine,
including the long file names:

--8<---------------cut here---------------start------------->8---
$ make dist -j5
[…]
make[2]: Leaving directory '/home/ludo/src/guix'
tardir=guix-1.3.0.8612-7cad44-dirty && tar --format=ustar -chf - "$tardir" | eval GZIP= gzip --best -c >guix-1.3.0.8612-7cad44-dirty.tar.gz
make[1]: Leaving directory '/home/ludo/src/guix'
if test -d "guix-1.3.0.8612-7cad44-dirty"; then find "guix-1.3.0.8612-7cad44-dirty" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -rf "guix-1.3.0.8612-7cad44-dirty" || { sleep 5 && rm -rf "guix-1.3.0.8612-7cad44-dirty"; }; else :; fi
$ tar tvf guix-1.3.0.8612-7cad44-dirty.tar.gz |grep patches/xplanet
-rw-r--r-- ludo/users     1252 2020-04-06 00:14 guix-1.3.0.8612-7cad44-dirty/gnu/packages/patches/xplanet-1.3.1-libimage_gif.c.patch
-rw-r--r-- ludo/users      765 2020-04-06 00:14 guix-1.3.0.8612-7cad44-dirty/gnu/packages/patches/xplanet-1.3.1-xpUtil-Add2017LeapSecond.cpp.patch
-rw-r--r-- ludo/users     6990 2020-04-06 00:14 guix-1.3.0.8612-7cad44-dirty/gnu/packages/patches/xplanet-1.3.1-cxx11-eof.patch
-rw-r--r-- ludo/users      783 2020-04-06 00:14 guix-1.3.0.8612-7cad44-dirty/gnu/packages/patches/xplanet-1.3.1-libdisplay_DisplayOutput.cpp.patch
--8<---------------cut here---------------end--------------->8---

> Seems a good thing to be as portable as can be, especially since 200
> chars patch file names wouldn't look good in the sources anyway ;-).

I don’t think we care about “portability” as understood in the context
of Automake, which roughly translates to “support SunOS 2 and HP-UX’s
1991 ‘tar’ implementation”.  :-)

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: default tar format for "make dist" and patch file length
  2021-11-22 11:31       ` Ludovic Courtès
@ 2021-11-22 20:14         ` Maxim Cournoyer
  0 siblings, 0 replies; 11+ messages in thread
From: Maxim Cournoyer @ 2021-11-22 20:14 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Vagrant Cascadian, guix-devel

Hi Ludo,

Ludovic Courtès <ludo@gnu.org> writes:

[...]

> Any objections against the patch below?
>
> diff --git a/configure.ac b/configure.ac
> index 84592f6041..341cff8fbd 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -8,7 +8,7 @@ AC_INIT([GNU Guix],
>    [https://www.gnu.org/software/guix/])
>  AC_CONFIG_AUX_DIR([build-aux])
>  
> -AM_INIT_AUTOMAKE([1.14 gnu silent-rules subdir-objects \
> +AM_INIT_AUTOMAKE([1.14 gnu tar-ustar silent-rules subdir-objects \
>   color-tests parallel-tests -Woverride -Wno-portability])
>  
>  # Enable silent rules by default.
>
>
> I tried it and there’s no tar warning and the file looks just fine,
> including the long file names:
>
> $ make dist -j5
> […]
> make[2]: Leaving directory '/home/ludo/src/guix'
> tardir=guix-1.3.0.8612-7cad44-dirty && tar --format=ustar -chf - "$tardir" | eval GZIP= gzip --best -c >guix-1.3.0.8612-7cad44-dirty.tar.gz
> make[1]: Leaving directory '/home/ludo/src/guix'
> if test -d "guix-1.3.0.8612-7cad44-dirty"; then find "guix-1.3.0.8612-7cad44-dirty" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -rf "guix-1.3.0.8612-7cad44-dirty" || { sleep 5 && rm -rf "guix-1.3.0.8612-7cad44-dirty"; }; else :; fi
> $ tar tvf guix-1.3.0.8612-7cad44-dirty.tar.gz |grep patches/xplanet
> -rw-r--r-- ludo/users     1252 2020-04-06 00:14 guix-1.3.0.8612-7cad44-dirty/gnu/packages/patches/xplanet-1.3.1-libimage_gif.c.patch
> -rw-r--r-- ludo/users      765 2020-04-06 00:14 guix-1.3.0.8612-7cad44-dirty/gnu/packages/patches/xplanet-1.3.1-xpUtil-Add2017LeapSecond.cpp.patch
> -rw-r--r-- ludo/users     6990 2020-04-06 00:14 guix-1.3.0.8612-7cad44-dirty/gnu/packages/patches/xplanet-1.3.1-cxx11-eof.patch
> -rw-r--r-- ludo/users      783 2020-04-06 00:14 guix-1.3.0.8612-7cad44-dirty/gnu/packages/patches/xplanet-1.3.1-libdisplay_DisplayOutput.cpp.patch
>
>> Seems a good thing to be as portable as can be, especially since 200
>> chars patch file names wouldn't look good in the sources anyway ;-).
>
> I don’t think we care about “portability” as understood in the context
> of Automake, which roughly translates to “support SunOS 2 and HP-UX’s
> 1991 ‘tar’ implementation”.  :-)

Hehe.

The patch LGTM.

Thanks!

Maxim


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: default tar format for "make dist" and patch file length
  2021-11-20  5:21           ` Vagrant Cascadian
@ 2021-11-24 21:27             ` Vagrant Cascadian
  0 siblings, 0 replies; 11+ messages in thread
From: Vagrant Cascadian @ 2021-11-24 21:27 UTC (permalink / raw)
  To: Philip McGrath, Ludovic Courtès; +Cc: guix-devel, 51985

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

On 2021-11-19, Vagrant Cascadian wrote:
> On 2021-11-19, Philip McGrath wrote:
>> On 11/19/21 09:54, Ludovic Courtès wrote:
>>> Vagrant Cascadian <vagrant@debian.org> skribis:
>>>> So, I guess I'm leaning towards making the guix lint check a little more
>>>> lenient.
>>>>
>>>> Thoughts?
>>> 
>>> That sounds even better, I’m all for it (changing (guix lint) + fixing
>>> the two remaining issues)!
>
> Submitted the guix lint change as https://issues.guix.gnu.org/51775

Er, I meant: https://issues.guix.gnu.org/51985


>> It might also help to change the warning given by the check.
>>
>> When a program called "lint" tells me that something is too long, I 
>> understand that to mean that what I've done is generally considered bad 
>> style, but there might be a very good reason to do it in some specific 
>> case. For example, I might exceed a line length guideline to avoid 
>> inserting linebreaks into a URL.
>
> That's a good point!
>
>
>> If instead `guix lint` is telling us about a hard limit that will break 
>> things, I think it should say so clearly.
>
> Not sure how to convey succinctly, but here's an attempt at a patch
> (which ironically also probably makes the line a bit too long in the
> code):
>
> diff --git a/guix/lint.scm b/guix/lint.scm
> index ac2e7b3841..6464fb751a 100644
> --- a/guix/lint.scm
> +++ b/guix/lint.scm
> @@ -968,7 +968,7 @@ (define (starts-with-package-name? file-name)
>                               max)
>                            (make-warning
>                             package
> -                           (G_ "~a: file name is too long")
> +                           (G_ "~a: file name is too long and may break release tarball generation")
>                             (list (basename patch))
>                             #:field 'patch-file-names)
>                            #f))
> diff --git a/tests/lint.scm b/tests/lint.scm
> index 9a91dd5426..d4c3d62aaf 100644
> --- a/tests/lint.scm
> +++ b/tests/lint.scm
> @@ -509,7 +509,7 @@ (define hsab (string-append (assoc-ref inputs
> "hsab")
>  (test-equal "patches: file name too long"
>    (string-append "x-"
>                   (make-string 100 #\a)
> -                 ".patch: file name is too long")
> +                 ".patch: file name is too long and may break release tarball generation")
>    (single-lint-warning-message
>     (let ((pkg (dummy-package
>                 "x"


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-11-24 21:28 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-16  0:34 default tar format for "make dist" and patch file length Vagrant Cascadian
2021-11-17 11:32 ` Ludovic Courtès
2021-11-17 22:39   ` Vagrant Cascadian
2021-11-17 23:49     ` Vagrant Cascadian
2021-11-19 14:54       ` Ludovic Courtès
2021-11-20  4:39         ` Philip McGrath
2021-11-20  5:21           ` Vagrant Cascadian
2021-11-24 21:27             ` Vagrant Cascadian
2021-11-22  2:03     ` Maxim Cournoyer
2021-11-22 11:31       ` Ludovic Courtès
2021-11-22 20:14         ` Maxim Cournoyer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).