[PATCH 0/1] gnu: icestorm: Replace reference in icebox_vlog.

[PATCH 0/1] gnu: icestorm: Replace reference in icebox_vlog.

[Theodoros Foradis]

Hello guix,

This patch fixes a reference to </usr/local/share/icebox/chipdb> in
icebox_vlog tool.

Regards,
-- 
Theodoros Foradis

[PATCH 1/1] gnu: icestorm: Replace reference in icebox_vlog.

[Theodoros Foradis]

* gnu/packages/fpga.scm (icestorm)[arguments]: Modify phases
"fix-usr-local" to replace reference to /usr/local/share.
---
 gnu/packages/fpga.scm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index f65eae8..eb05a39 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -225,11 +225,12 @@ For synthesis, the compiler generates netlists in the desired format.")
                           (string-append "PREFIX=" (assoc-ref %outputs "out")))
        #:phases
         (modify-phases %standard-phases
-          (add-after 'unpack 'remove-usr-local
-            (lambda _
-              (substitute* "iceprog/Makefile"
+          (add-after 'unpack 'fix-usr-local
+            (lambda* (#:key outputs #:allow-other-keys)
+              (substitute* '("iceprog/Makefile" "icebox/icebox_vlog.py")
                 (("-I/usr/local/include") "")
-                (("-L/usr/local/lib") ""))
+                (("-L/usr/local/lib") "")
+                (("/usr/local/share") (string-append (assoc-ref outputs "out") "/share")))
               #t))
           (delete 'configure))))
     (inputs
-- 
2.10.2

Re: [PATCH 1/1] gnu: icestorm: Replace reference in icebox_vlog.

[Danny Milosavljevic]

Hi,

thanks for the patch!

On Sat, 14 Jan 2017 20:56:52 +0200
Theodoros Foradis <theodoros.for@openmailbox.org> wrote:

> * gnu/packages/fpga.scm (icestorm)[arguments]: Modify phases
> "fix-usr-local" to replace reference to /usr/local/share.
> ---
>  gnu/packages/fpga.scm | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
> index f65eae8..eb05a39 100644
> --- a/gnu/packages/fpga.scm
> +++ b/gnu/packages/fpga.scm
> @@ -225,11 +225,12 @@ For synthesis, the compiler generates netlists in the desired format.")
>                            (string-append "PREFIX=" (assoc-ref %outputs "out")))
>         #:phases
>          (modify-phases %standard-phases
> -          (add-after 'unpack 'remove-usr-local
> -            (lambda _
> -              (substitute* "iceprog/Makefile"
> +          (add-after 'unpack 'fix-usr-local
> +            (lambda* (#:key outputs #:allow-other-keys)

> +              (substitute* '("iceprog/Makefile"

Patching this files changes nothing in the final result.

The package is built in an isolated container anyway - it's not like it can access /usr/local while it's building.

And if we did that everywhere we would have lots of substitution commands all over the place as noise - doing nothing useful, drowning out the important signal of actually useful substitutions. Therefore, I don't think we should do this.

What do the others think?

> "icebox/icebox_vlog.py")

... oops. This one I definitely agree with (the /usr/local/share one).

Re: [PATCH 1/1] gnu: icestorm: Replace reference in icebox_vlog.

[Theodoros Foradis]

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


Danny Milosavljevic writes:

> Hi,
>
> thanks for the patch!
>
> On Sat, 14 Jan 2017 20:56:52 +0200
> Theodoros Foradis <theodoros.for@openmailbox.org> wrote:
>
>> * gnu/packages/fpga.scm (icestorm)[arguments]: Modify phases
>> "fix-usr-local" to replace reference to /usr/local/share.
>> ---
>>  gnu/packages/fpga.scm | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>> 
>> diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
>> index f65eae8..eb05a39 100644
>> --- a/gnu/packages/fpga.scm
>> +++ b/gnu/packages/fpga.scm
>> @@ -225,11 +225,12 @@ For synthesis, the compiler generates netlists in the desired format.")
>>                            (string-append "PREFIX=" (assoc-ref %outputs "out")))
>>         #:phases
>>          (modify-phases %standard-phases
>> -          (add-after 'unpack 'remove-usr-local
>> -            (lambda _
>> -              (substitute* "iceprog/Makefile"
>> +          (add-after 'unpack 'fix-usr-local
>> +            (lambda* (#:key outputs #:allow-other-keys)
>
>> +              (substitute* '("iceprog/Makefile"
>
> Patching this files changes nothing in the final result.
>
> The package is built in an isolated container anyway - it's not like it can access /usr/local while it's building.
>
> And if we did that everywhere we would have lots of substitution commands all over the place as noise - doing nothing useful, drowning out the important signal of actually useful substitutions. Therefore, I don't think we should do this.
>
> What do the others think?
>
>> "icebox/icebox_vlog.py")
>
> ... oops. This one I definitely agree with (the /usr/local/share one).

I should have probably added a different phase for the
"icebox/icebox_vlog.py" substitution, to make clear it's the only path
which is a run-time reference. The installed icebox_vlog, can't find the
chipdb.txt file, if we don't make the substitution.

I attach the modified patch, adding a new phase instead.

Regards,
-- 
Theodoros Foradis

[-- Attachment #2: 0001-gnu-icestorm-Replace-reference-in-icebox_vlog.patch --]
[-- Type: text/plain, Size: 1140 bytes --]

From 5055cf57375103f2d01f286d9b2615defea20968 Mon Sep 17 00:00:00 2001
From: Theodoros Foradis <theodoros.for@openmailbox.org>
Date: Sat, 14 Jan 2017 20:31:34 +0200
Subject: [PATCH v2 1/1] gnu: icestorm: Replace reference in icebox_vlog.

* gnu/packages/fpga.scm (icestorm)[arguments]: Add phase
"fix-usr-local" to replace reference to /usr/local/share.
---
 gnu/packages/fpga.scm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gnu/packages/fpga.scm b/gnu/packages/fpga.scm
index f65eae8..0b98e2d 100644
--- a/gnu/packages/fpga.scm
+++ b/gnu/packages/fpga.scm
@@ -231,6 +231,11 @@ For synthesis, the compiler generates netlists in the desired format.")
                 (("-I/usr/local/include") "")
                 (("-L/usr/local/lib") ""))
               #t))
+          (add-after 'remove-usr-local 'fix-usr-local
+            (lambda* (#:key outputs #:allow-other-keys)
+              (substitute* "icebox/icebox_vlog.py"
+                (("/usr/local/share") (string-append (assoc-ref outputs "out") "/share")))
+              #t))
           (delete 'configure))))
     (inputs
      `(("libftdi" ,libftdi)))
-- 
2.10.2

Re: [PATCH 1/1] gnu: icestorm: Replace reference in icebox_vlog.

[Danny Milosavljevic]

Hi,

> I should have probably added a different phase for the
> "icebox/icebox_vlog.py" substitution, to make clear it's the only path
> which is a run-time reference. The installed icebox_vlog, can't find the
> chipdb.txt file, if we don't make the substitution.

Yeah, I think how you did it now is best.

LGTM!

I've built it and it works.

I'll leave some time for others to comment. Afterwards, I'll commit it to master (since it'll cause rebuild of one icestorm-dependent package).

Re: [PATCH 1/1] gnu: icestorm: Replace reference in icebox_vlog.

[Leo Famulari]

On Mon, Jan 16, 2017 at 03:21:29PM +0100, Danny Milosavljevic wrote:
> Hi,
> 
> > I should have probably added a different phase for the
> > "icebox/icebox_vlog.py" substitution, to make clear it's the only path
> > which is a run-time reference. The installed icebox_vlog, can't find the
> > chipdb.txt file, if we don't make the substitution.
> 
> Yeah, I think how you did it now is best.
> 
> LGTM!
> 
> I've built it and it works.
> 
> I'll leave some time for others to comment. Afterwards, I'll commit it
> to master (since it'll cause rebuild of one icestorm-dependent
> package).

LGTM! Thanks Theodoros and Danny!

Re: [PATCH 1/1] gnu: icestorm: Replace reference in icebox_vlog.

[Theodoros Foradis]

Leo Famulari writes:

> On Mon, Jan 16, 2017 at 03:21:29PM +0100, Danny Milosavljevic wrote:
>> Hi,
>> 
>> > I should have probably added a different phase for the
>> > "icebox/icebox_vlog.py" substitution, to make clear it's the only path
>> > which is a run-time reference. The installed icebox_vlog, can't find the
>> > chipdb.txt file, if we don't make the substitution.
>> 
>> Yeah, I think how you did it now is best.
>> 
>> LGTM!
>> 
>> I've built it and it works.
>> 
>> I'll leave some time for others to comment. Afterwards, I'll commit it
>> to master (since it'll cause rebuild of one icestorm-dependent
>> package).
>
> LGTM! Thanks Theodoros and Danny!

Hello,

Just a reminder for this patch, because I see that it has not been
commited yet. Is there anything blocking it?

Thanks,
-- 
Theodoros Foradis

Re: [PATCH 1/1] gnu: icestorm: Replace reference in icebox_vlog.

[Danny Milosavljevic]

Sorry, the previously attached patch didn't apply using "git am" so I forgot about it. 

I fixed it now and commited it as b500dc2e83adc5529b890349af48655d0adaf1f8 to master.