[bug#31016] [PATCH] gnu: git: Fix checksum patch in 'patch-tests'.

[bug#31016] [PATCH] gnu: git: Fix checksum patch in 'patch-tests'.

[YOANN P]

Hi Guix Team,

First contribution to Guix project so i hope i forgot nothing in the process to submit my patch.
There is already some patchs for "t/t9100-git-svn-basic.sh" and "t/t9300-fast-import.sh" but they 
assume than the store is always "/gnu/store".
The bellow patch is intend to correct this and lets the check phase work the same way if a custom 
store is used.

Best regards,


From 08b8d3b9d32bd7f3f5b762541f38f95a2eb63c2a Mon Sep 17 00:00:00 2001
From: RockAndSka <yoann_mac_donald@hotmail.com>
Date: Sun, 1 Apr 2018 16:11:30 +0200
Subject: [PATCH] gnu: git: Fix checksum patch in 'patch-tests'.

* gnu/packages/version-control.scm (git)[arguments]: In 'patch-tests'
phase, use %store-directory instead of '/gnu' to prevent tests failure
in case a custom store path is used.
---
 gnu/packages/version-control.scm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/version-control.scm b/gnu/packages/version-control.scm
index ba985f6..60a509e 100644
--- a/gnu/packages/version-control.scm
+++ b/gnu/packages/version-control.scm
@@ -238,10 +238,10 @@ as well as the classic centralized workflow.")
               (("\tcommit_template_commented") "\ttrue"))
             ;; More checksum mismatches due to odd shebangs.
             (substitute* "t/t9100-git-svn-basic.sh"
-              (("\"#!/gnu.*/bin/sh") "\"#!/bin/sh"))
+              (((string-append "\"#!" (%store-directory) ".*/bin/sh")) "\"#!/bin/sh") )
             (substitute* "t/t9300-fast-import.sh"
-              (("\t#!/gnu.*/bin/sh") "\t#!/bin/sh")
-              (("'#!/gnu.*/bin/sh") "'#!/bin/sh"))
+              (((string-append "\t#!" (%store-directory) ".*/bin/sh")) "\t#!/bin/sh")
+              (((string-append "'#!" (%store-directory) ".*/bin/sh")) "'#!/bin/sh"))
             ;; FIXME: Some hooks fail with "basename: command not found".
             ;; See 't/trash directory.t9164.../svn-hook.log'.
             (delete-file "t/t9164-git-svn-dcommit-concurrent.sh")
-- 
2.7.4

[bug#31016] [PATCH] gnu: git: Fix checksum patch in 'patch-tests'.

[Marius Bakke]

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

Hi Yoann,

YOANN P <yoann_mac_donald@hotmail.com> writes:

> Hi Guix Team,
>
> First contribution to Guix project so i hope i forgot nothing in the process to submit my patch.
> There is already some patchs for "t/t9100-git-svn-basic.sh" and "t/t9300-fast-import.sh" but they 
> assume than the store is always "/gnu/store".
> The bellow patch is intend to correct this and lets the check phase work the same way if a custom 
> store is used.

Thank you for this patch!

[...]

> * gnu/packages/version-control.scm (git)[arguments]: In 'patch-tests'
> phase, use %store-directory instead of '/gnu' to prevent tests failure
> in case a custom store path is used.

[...]

> @@ -238,10 +238,10 @@ as well as the classic centralized workflow.")
>                (("\tcommit_template_commented") "\ttrue"))
>              ;; More checksum mismatches due to odd shebangs.
>              (substitute* "t/t9100-git-svn-basic.sh"
> -              (("\"#!/gnu.*/bin/sh") "\"#!/bin/sh"))
> +              (((string-append "\"#!" (%store-directory) ".*/bin/sh")) "\"#!/bin/sh") )
>              (substitute* "t/t9300-fast-import.sh"
> -              (("\t#!/gnu.*/bin/sh") "\t#!/bin/sh")
> -              (("'#!/gnu.*/bin/sh") "'#!/bin/sh"))
> +              (((string-append "\t#!" (%store-directory) ".*/bin/sh")) "\t#!/bin/sh")
> +              (((string-append "'#!" (%store-directory) ".*/bin/sh")) "'#!/bin/sh"))

Calling out to (%store-directory) inside a string substitution multiple
times is not great.  Can you try wrapping this phase in a let binding
that expands (%store-directory) once, and use that?

Something along the lines of:

(add-before 'check 'patch-tests
 (lambda _
  (let ((store-directory (%store-directory)))
   [...]
   (string-append "..." store-directory ".*/bin/sh"))))

We'll have to reindent it, but that's okay.  Can you send an updated
patch?

Thanks in advance, and welcome!

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

[bug#31016] [PATCH] gnu: git: Fix checksum patch in 'patch-tests'.

[YOANN P]

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

>Hi Yoann,
>
>

Hi Marius,

And thanks again for yesterday on irc to point me at how to solve this bug ^^

>YOANN P <yoann_mac_donald@hotmail.com> writes:
>
>> Hi Guix Team,
>>
>> First contribution to Guix project so i hope i forgot nothing in the process to submit my patch.
>> There is already some patchs for "t/t9100-git-svn-basic.sh" and "t/t9300-fast-import.sh" but they
>> assume than the store is always "/gnu/store".
>> The bellow patch is intend to correct this and lets the check phase work the same way if a custom
>> store is used.
>
>Thank you for this patch!
>
>[...]
>
>> * gnu/packages/version-control.scm (git)[arguments]: In 'patch-tests'
>> phase, use %store-directory instead of '/gnu' to prevent tests failure
>> in case a custom store path is used.
>
>[...]
>
>> @@ -238,10 +238,10 @@ as well as the classic centralized workflow.")
>>                (("\tcommit_template_commented") "\ttrue"))
>>              ;; More checksum mismatches due to odd shebangs.
>>              (substitute* "t/t9100-git-svn-basic.sh"
>> -              (("\"#!/gnu.*/bin/sh") "\"#!/bin/sh"))
>> +              (((string-append "\"#!" (%store-directory) ".*/bin/sh")) "\"#!/bin/sh") )
>>              (substitute* "t/t9300-fast-import.sh"
>> -              (("\t#!/gnu.*/bin/sh") "\t#!/bin/sh")
>> -              (("'#!/gnu.*/bin/sh") "'#!/bin/sh"))
>> +              (((string-append "\t#!" (%store-directory) ".*/bin/sh")) "\t#!/bin/sh")
>> +              (((string-append "'#!" (%store-directory) ".*/bin/sh")) "'#!/bin/sh"))
>
>Calling out to (%store-directory) inside a string substitution multiple
>times is not great.  Can you try wrapping this phase in a let binding
>that expands (%store-directory) once, and use that?
>
>Something along the lines of:
>
>(add-before 'check 'patch-tests
> (lambda _
>  (let ((store-directory (%store-directory)))
>   [...]
>   (string-append "..." store-directory ".*/bin/sh"))))
>
>We'll have to reindent it, but that's okay.  Can you send an updated
>patch?

Sorry for the indentation, I indeed used my VIMorite editor and don't use Emacs to be able to use the indentation plugin :/
Please find bellow the patch modified with the modifications asked ( I just recompiled Git with those modifications to be sure and seems ok)

----

From 3df3fbfadf24b2521a1b60ea853d7fcec6452f44 Mon Sep 17 00:00:00 2001
From: RockAndSka <yoann_mac_donald@hotmail.com>
Date: Sun, 1 Apr 2018 16:11:30 +0200
Subject: [PATCH] gnu: git: Fix checksum patch in 'patch-tests'.

* gnu/packages/version-control.scm (git)[arguments]: In 'patch-tests'
phase, use %store-directory instead of '/gnu' to prevent tests failure
in case a custom store path is used.
---
 gnu/packages/version-control.scm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/version-control.scm b/gnu/packages/version-control.scm
index ba985f6..619052d 100644
--- a/gnu/packages/version-control.scm
+++ b/gnu/packages/version-control.scm
@@ -217,6 +217,7 @@ as well as the classic centralized workflow.")
             #t))
         (add-before 'check 'patch-tests
           (lambda _
+     (let ((store-directory (%store-directory)))
             ;; These files contain some funny bytes that Guile is unable
             ;; to decode for shebang patching. Just delete them.
             (for-each delete-file '("t/t4201-shortlog.sh"
@@ -238,10 +239,10 @@ as well as the classic centralized workflow.")
               (("\tcommit_template_commented") "\ttrue"))
             ;; More checksum mismatches due to odd shebangs.
             (substitute* "t/t9100-git-svn-basic.sh"
-              (("\"#!/gnu.*/bin/sh") "\"#!/bin/sh"))
+              (((string-append "\"#!" store-directory ".*/bin/sh")) "\"#!/bin/sh") )
             (substitute* "t/t9300-fast-import.sh"
-              (("\t#!/gnu.*/bin/sh") "\t#!/bin/sh")
-              (("'#!/gnu.*/bin/sh") "'#!/bin/sh"))
+              (((string-append "\t#!" store-directory ".*/bin/sh")) "\t#!/bin/sh")
+              (((string-append "'#!" store-directory ".*/bin/sh")) "'#!/bin/sh"))
             ;; FIXME: Some hooks fail with "basename: command not found".
             ;; See 't/trash directory.t9164.../svn-hook.log'.
             (delete-file "t/t9164-git-svn-dcommit-concurrent.sh")
@@ -252,7 +253,7 @@ as well as the classic centralized workflow.")
                       '("t/t9128-git-svn-cmd-branch.sh"
                         "t/t9167-git-svn-cmd-branch-subproject.sh"
                         "t/t9141-git-svn-multiple-branches.sh"))
-            #t))
+            #t)))
         (add-after 'install 'install-shell-completion
           (lambda* (#:key outputs #:allow-other-keys)
             (let* ((out         (assoc-ref outputs "out"))
--
2.7.4



>
>Thanks in advance, and welcome!

Thanks to you and all the hard work provided by people like you to provide us tools like Guix :)

Let me know if I missed something

Regards,

[-- Attachment #2: Type: text/html, Size: 9497 bytes --]

bug#31016: [PATCH] gnu: git: Fix checksum patch in 'patch-tests'.

[Marius Bakke]

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

YOANN P <yoann_mac_donald@hotmail.com> writes:

>>We'll have to reindent it, but that's okay.  Can you send an updated
>>patch?
>
> Sorry for the indentation, I indeed used my VIMorite editor and don't use Emacs to be able to use the indentation plugin :/

There is a script you can run to indent even if you don't use emacs.
From a git checkout, you can do:
"./etc/indent-code.el gnu/packages/version-control.scm git".

Unfortunately, "git" is currently wrongly indented at the (package ..)
level, so it would reindent the whole package instead of just the phase.

> Please find bellow the patch modified with the modifications asked ( I just recompiled Git with those modifications to be sure and seems ok)

Thank you!  It works for me as well.  Pushed as 6e0efe8cd!

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