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

  • Done
  • quality assurance status badge
Details
2 participants
  • Marius Bakke
  • YOANN P
Owner
unassigned
Submitted by
YOANN P
Severity
normal
Y
Y
YOANN P wrote on 1 Apr 2018 17:03
(name . guix-patches@gnu.org)(address . guix-patches@gnu.org)
DB6P18901MB002290D26A0CEE6A0BF33D3ADBA70@DB6P18901MB0022.EURP189.PROD.OUTLOOK.COM
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(-)

Toggle diff (20 lines)
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
M
M
Marius Bakke wrote on 3 Apr 2018 12:42
87h8osbmfk.fsf@fastmail.com
Hi Yoann,

YOANN P <yoann_mac_donald@hotmail.com> writes:

Toggle quote (8 lines)
> 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!

[...]

Toggle quote (4 lines)
> * 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.

[...]

Toggle quote (12 lines)
> @@ -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!
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAlrDWq8ACgkQoqBt8qM6
VPrtYwf+IZVum6Yvio41zvYAtY45ID7WXYqNKwyfDZlWiyAKzEnqOXP2PJ4CiRiT
NuhMlbyR8Uwp4xHm8cOXgclcMbBZm8Hzwku/WWuuGkKQU7QtGoG75Ybh7cYdrdOv
4GTSpeWGwNfsm9hlHBA9kGf6REmrd6RfRpNs7Allfl5MoUv9yo1I9Y9r0QbP6GGV
pkD8W2aNvkV2DGb2FCQjEzQb+lxG9u8HLdX65pbIlXd+hBcRPkWKjJymNDYSHuQh
PaLxq9LHHvf/3YJabviTcb7+31Y+rcA+XrUVz37MxidDLu9dBwnrf+gcDb0ka14m
cj0Km47Y7t4zLYMJ5z4k8qNrb75ndA==
=0FOT
-----END PGP SIGNATURE-----

Y
Y
YOANN P wrote on 3 Apr 2018 20:05
RE: [bug#31016] [PATCH] gnu: git: Fix checksum patch in 'patch-tests'.
DB6P18901MB002280383A8DB2C3FC4AC9B3DBA50@DB6P18901MB0022.EURP189.PROD.OUTLOOK.COM
Toggle quote (4 lines)
>Hi Yoann,
>
>

Hi Marius,

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

Toggle quote (47 lines)
>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(-)

Toggle diff (35 lines)
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



Toggle quote (3 lines)
>
>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: file
M
M
Marius Bakke wrote on 6 Apr 2018 20:22
87a7ugyz2e.fsf@fastmail.com
YOANN P <yoann_mac_donald@hotmail.com> writes:

Toggle quote (5 lines)
>>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.

Toggle quote (2 lines)
> 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!
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAlrHuvkACgkQoqBt8qM6
VPqpTQf/csPfz4Bm/+cn36qmzjopKT3AfcwXg7JSoCacUUydPOuAhBusaK5jasvm
0h9qENHKRGiAPv83uMYNFmq8gQl8cOQg9EhAgS20L+pmych8VUcRxl5RbfGjkO31
s19Hrr89GahF8OKft07n82vht0ddV1CBkh3/BEVLWY38ge4YLmNy/HeDLQC5Mqe3
5aoJjRl+My1LhLVV8HP+uqz7rHhb3Bg2VNbGOb1Moy4I43itLLGnJ52BtHJZmPrJ
3WS+76nmqKIn1ndtlCsVqtAVrr64YoffWxVzBlGjrijs7AlaCKHVbkpSsr/juo3S
QX03q0Z7p1yb+AL8KTVcTlWgvcaixQ==
=ujys
-----END PGP SIGNATURE-----

Closed
?