Subject: Re: bug#49149: [PATCH 0/7] Add deb format for guix pack.

From: Maxim Cournoyer <[email-masked]>
Date: Sat, 03 Jul 2021 23:21:05 -0400

Hi!

Ludovic Courtès <[email-masked]> writes:

> Hi,
>
> Maxim Cournoyer <[email-masked]> skribis:
>
>> Ludovic Courtès <[email-masked]> writes:
>>
>>> Maxim Cournoyer <[email-masked]> skribis:
>>>
>>>> Instead of just naming them by their pack type, add information from the
>>>> package(s) they contain to make it easier to differentiate them.
>>>>
>>>> * guix/scripts/pack.scm (define-with-source): New macro.
>>>> (manifest->friendly-name): Extract procedure from ...
>>>> (docker-image): ... here, now defined via the above macro.  Adjust REPOSITORY
>>>> argument value accordingly.
>>>> (guix-pack): Derive NAME using MANIFEST->FRIENDLY-NAME.
>>>
>>> [...]
>>>
>>>> -            (define tag
>>>> -              ;; Compute a meaningful "repository" name, which will show up in
>>>> -              ;; the output of "docker images".
>>>> -              (let ((manifest (profile-manifest #$profile)))
>>>> -                (let loop ((names (map manifest-entry-name
>>>> -                                       (manifest-entries manifest))))
>>>> -                  (define str (string-join names "-"))
>>>> -                  (if (< (string-length str) 40)
>>>> -                      str
>>>> -                      (match names
>>>> -                        ((_) str)
>>>> -                        ((names ... _) (loop names))))))) ;drop one entry
>>>
>>> I think this should not be factorized because the requirements are very
>>> Docker-dependent.  Once factorized, it becomes easy to overlook this.
>>
>> Hmm, I'm not a docker format expert, but my quick reading about it
>> turned no restrictions about what a docker image label should look like?
>> So perhaps it is not specially Docker-dependent.
>
> It’s a hack specifically written with Docker repository names in mind,
> and the 40-or-so character limit, for instance.

The actual name length requirement for a Docker repository name seems to
be that it must be between 2 and 255 characters [0]; the attached patch
ensure that this is respected.

> To me it’s a case where factorization isn’t beneficial.  Even if there’s
> a similar procedure used in a different context, it’s still a different
> context with different constraints.  My 2¢!

It seems to me that with the attached patch we get to share what used to
be a Docker-specific abstraction without any added risk (have our cake
and it eat to!).

What do you think?

Thanks,

Maxim

>From f3dc90213423bf0a087245bd4bfc8c4a828d4df1 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <[email-masked]>
Date: Sat, 3 Jul 2021 23:08:15 -0400
Subject: [PATCH] guix: docker: Ensure repository name length limits are met.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* guix/docker.scm (canonicalize-repository-name): Fix typo in doc.  Capture
repository name length limits and ensure they are met, by either truncating or
padding the normalized name.

Reported-by: Ludovic Courtès <[email-masked]>
---
 guix/docker.scm | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/guix/docker.scm b/guix/docker.scm
index bd952e45ec..4239ccdf9c 100644
--- a/guix/docker.scm
+++ b/guix/docker.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2017 Ricardo Wurmus <[email-masked]>
 ;;; Copyright © 2017, 2018, 2019, 2021 Ludovic Courtès <[email-masked]>
 ;;; Copyright © 2018 Chris Marusich <[email-masked]>
+;;; Copyright © 2021 Maxim Cournoyer <[email-masked]>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -59,8 +60,13 @@
     (container_config . #nil)))
 
 (define (canonicalize-repository-name name)
-  "\"Repository\" names are restricted to roughtl [a-z0-9_.-].
+  "\"Repository\" names are restricted to roughly [a-z0-9_.-].
 Return a version of TAG that follows these rules."
+  ;; Refer to https://docs.docker.com/docker-hub/repos/.
+  (define min-length 2)
+  (define padding-character #\a)
+  (define max-length 255)
+
   (define ascii-letters
     (string->char-set "abcdefghijklmnopqrstuvwxyz"))
 
@@ -70,11 +76,21 @@ Return a version of TAG that follows these rules."
   (define repo-char-set
     (char-set-union char-set:digit ascii-letters separators))
 
-  (string-map (lambda (chr)
-                (if (char-set-contains? repo-char-set chr)
-                    chr
-                    #\.))
-              (string-trim (string-downcase name) separators)))
+  (define normalized-name
+    (string-map (lambda (chr)
+                  (if (char-set-contains? repo-char-set chr)
+                      chr
+                      #\.))
+                (string-trim (string-downcase name) separators)))
+
+  (let ((l (string-length normalized-name)))
+    (match l
+      ((? (cut > <> max-length))
+       (string-take normalized-name max-length))
+      ((? (cut < <> min-length ))
+       (string-append normalized-name
+                      (make-string (- min-length l) padding-character)))
+      (_ normalized-name))))
 
 (define* (manifest path id #:optional (tag "guix"))
   "Generate a simple image manifest."
-- 
2.32.0