diff mbox

[bug#49149,0/7] Add deb format for guix pack.

Message ID 87zgv2228u.fsf@gmail.com
State Accepted
Headers show

Commit Message

Maxim Cournoyer July 4, 2021, 3:21 a.m. UTC 
Hi!

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

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Maxim Cournoyer <maxim.cournoyer@gmail.com> 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

Comments

Ludovic Courtès July 5, 2021, 4:14 p.m. UTC | #1 
Hello,

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

[...]

>> 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!).

[...]

> From f3dc90213423bf0a087245bd4bfc8c4a828d4df1 Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> 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 <ludo@gnu.org>

LGTM, thank you!

Ludo’.
Maxim Cournoyer July 5, 2021, 8:42 p.m. UTC | #2 
Hello,

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

[...]

>> From f3dc90213423bf0a087245bd4bfc8c4a828d4df1 Mon Sep 17 00:00:00 2001
>> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>> 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 <ludo@gnu.org>
>
> LGTM, thank you!
>
> Ludo’.

Pushed as 38bcef1c3b.

Thanks!

Maxim
diff mbox

Patch

From f3dc90213423bf0a087245bd4bfc8c4a828d4df1 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
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 <ludo@gnu.org>
---
 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 <rekado@elephly.net>
 ;;; Copyright © 2017, 2018, 2019, 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2018 Chris Marusich <cmmarusich@gmail.com>
+;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;;
 ;;; 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