all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [PATCH] gnu: dub: Patch pkg-config name.
@ 2017-01-31 16:00 Danny Milosavljevic
  2017-02-01 22:22 ` Ludovic Courtès
  0 siblings, 1 reply; 7+ messages in thread
From: Danny Milosavljevic @ 2017-01-31 16:00 UTC (permalink / raw)
  To: guix-devel

* gnu/packages/ldc.scm (dub)[arguments]: Add 'patch-paths' phase.
---
 gnu/packages/ldc.scm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/ldc.scm b/gnu/packages/ldc.scm
index 4b10ac25e..e131c473d 100644
--- a/gnu/packages/ldc.scm
+++ b/gnu/packages/ldc.scm
@@ -32,6 +32,7 @@
   #:use-module (gnu packages gdb)
   #:use-module (gnu packages libedit)
   #:use-module (gnu packages llvm)
+  #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages python)
   #:use-module (gnu packages textutils)
   #:use-module (gnu packages zip))
@@ -293,6 +294,12 @@ latest DMD frontend and uses LLVM as backend.")
      `(#:tests? #f ; it would have tested itself by installing some packages (vibe etc)
        #:phases
        (modify-phases %standard-phases
+         (add-after 'unpack 'patch-paths
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (substitute* "source/dub/compilers/utils.d"
+               ; TODO patch source/dub/platform.d compiler executable name ??
+               (("enum pkgconfig_bin = \"pkg-config\";") (string-append "enum pkgconfig_bin = \"" (assoc-ref inputs "pkg-config") "/bin/pkg-config\";")))
+             #t))
          (delete 'configure)
          (replace 'build
            (lambda _
@@ -305,7 +312,8 @@ latest DMD frontend and uses LLVM as backend.")
                (install-file "bin/dub" outbin)
                #t))))))
     (inputs
-     `(("curl" ,curl)))
+     `(("curl" ,curl)
+       ("pkg-config" ,pkg-config)))
     (native-inputs
      `(("ldc" ,ldc)))
     (home-page "https://code.dlang.org/getting_started")

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] gnu: dub: Patch pkg-config name.
  2017-01-31 16:00 [PATCH] gnu: dub: Patch pkg-config name Danny Milosavljevic
@ 2017-02-01 22:22 ` Ludovic Courtès
  2017-02-02 19:36   ` Danny Milosavljevic
  0 siblings, 1 reply; 7+ messages in thread
From: Ludovic Courtès @ 2017-02-01 22:22 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: guix-devel

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> * gnu/packages/ldc.scm (dub)[arguments]: Add 'patch-paths' phase.
> ---
>  gnu/packages/ldc.scm | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/ldc.scm b/gnu/packages/ldc.scm
> index 4b10ac25e..e131c473d 100644
> --- a/gnu/packages/ldc.scm
> +++ b/gnu/packages/ldc.scm
> @@ -32,6 +32,7 @@
>    #:use-module (gnu packages gdb)
>    #:use-module (gnu packages libedit)
>    #:use-module (gnu packages llvm)
> +  #:use-module (gnu packages pkg-config)
>    #:use-module (gnu packages python)
>    #:use-module (gnu packages textutils)
>    #:use-module (gnu packages zip))
> @@ -293,6 +294,12 @@ latest DMD frontend and uses LLVM as backend.")
>       `(#:tests? #f ; it would have tested itself by installing some packages (vibe etc)
>         #:phases
>         (modify-phases %standard-phases
> +         (add-after 'unpack 'patch-paths
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (substitute* "source/dub/compilers/utils.d"
> +               ; TODO patch source/dub/platform.d compiler executable name ??
> +               (("enum pkgconfig_bin = \"pkg-config\";") (string-append "enum pkgconfig_bin = \"" (assoc-ref inputs "pkg-config") "/bin/pkg-config\";")))
> +             #t))

Is it necessary?  It might be a case where picking whatever’s in $PATH
(“late binding”) is good enough; for instance, ‘gcc’ picks ‘ld’ from
$PATH, and that’s fine.

WDYT?

(Besides, shorter lines please, and s/path/file name/.)

Ludo’.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] gnu: dub: Patch pkg-config name.
  2017-02-01 22:22 ` Ludovic Courtès
@ 2017-02-02 19:36   ` Danny Milosavljevic
  2017-02-03 15:14     ` Danny Milosavljevic
  2017-02-03 16:49     ` Ludovic Courtès
  0 siblings, 2 replies; 7+ messages in thread
From: Danny Milosavljevic @ 2017-02-02 19:36 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

Hi Ludo,

On Wed, 01 Feb 2017 23:22:32 +0100
ludo@gnu.org (Ludovic Courtès) wrote:

> Is it necessary?  It might be a case where picking whatever’s in $PATH
> (“late binding”) is good enough; for instance, ‘gcc’ picks ‘ld’ from
> $PATH, and that’s fine.

I don't know. What's the policy for pkg-config in general?

I did it that way now because

(1) pkg-config is not a native-input of anything D - so if the user didn't install pkg-config into his profile by chance, the build of some D packages will fail.
(2) pkg-config itself is very seldomly updated so it doesn't matter if dub pins it to one specific version - that would even be what I would expect to happen.

I'm fine with adding it someway else (especially if it's the same way it's usually added).

So either

(a) dub needs it as input (like this patch would do) or
(b) every D package needs it as native-input (maybe the build system would auto-add it as native-input to every D package - like it does add ldc and dub)

- otherwise it will break.

(dub uses pkg-config in order to find the library flags to add when importing a given library)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] gnu: dub: Patch pkg-config name.
  2017-02-02 19:36   ` Danny Milosavljevic
@ 2017-02-03 15:14     ` Danny Milosavljevic
  2017-02-03 16:49     ` Ludovic Courtès
  1 sibling, 0 replies; 7+ messages in thread
From: Danny Milosavljevic @ 2017-02-03 15:14 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

>So either
> (a) dub needs it as input (like this patch would do) or
> (b) every D package needs it as native-input (maybe the build system would auto-add it as native-input to every D package - like it does add ldc and dub)

(c) dub propagates pkg-config

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] gnu: dub: Patch pkg-config name.
  2017-02-02 19:36   ` Danny Milosavljevic
  2017-02-03 15:14     ` Danny Milosavljevic
@ 2017-02-03 16:49     ` Ludovic Courtès
  2017-02-03 17:38       ` Danny Milosavljevic
  1 sibling, 1 reply; 7+ messages in thread
From: Ludovic Courtès @ 2017-02-03 16:49 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: guix-devel

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> Hi Ludo,
>
> On Wed, 01 Feb 2017 23:22:32 +0100
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> Is it necessary?  It might be a case where picking whatever’s in $PATH
>> (“late binding”) is good enough; for instance, ‘gcc’ picks ‘ld’ from
>> $PATH, and that’s fine.
>
> I don't know. What's the policy for pkg-config in general?
>
> I did it that way now because
>
> (1) pkg-config is not a native-input of anything D - so if the user didn't install pkg-config into his profile by chance, the build of some D packages will fail.
> (2) pkg-config itself is very seldomly updated so it doesn't matter if dub pins it to one specific version - that would even be what I would expect to happen.
>
> I'm fine with adding it someway else (especially if it's the same way it's usually added).
>
> So either
>
> (a) dub needs it as input (like this patch would do) or
> (b) every D package needs it as native-input (maybe the build system would auto-add it as native-input to every D package - like it does add ldc and dub)
>
> - otherwise it will break.

I think ‘dub-build-system’ could add it as an implicit input, much like
‘gnu-build-system’ adds binutils as an implicit input.

Or we could simply let people add pkg-config as an input when it’s
necessary, just like we do for ‘gnu-build-system’ packages.

Thoughts?

Ludo’.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] gnu: dub: Patch pkg-config name.
  2017-02-03 16:49     ` Ludovic Courtès
@ 2017-02-03 17:38       ` Danny Milosavljevic
  2017-02-07 14:13         ` Ludovic Courtès
  0 siblings, 1 reply; 7+ messages in thread
From: Danny Milosavljevic @ 2017-02-03 17:38 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

Hi Ludo,

On Fri, 03 Feb 2017 17:49:38 +0100
ludo@gnu.org (Ludovic Courtès) wrote:

> I think ‘dub-build-system’ could add it as an implicit input, much like
> ‘gnu-build-system’ adds binutils as an implicit input.

Okay, but it's directly used only by dub (it its function of building D packages).

I don't think D packages themselves even know what pkg-config is.

The ldc 1.1.0 sources don't even mention "pkg-config" once - neither do any of the D packages I tried except gtk-d. That one mentions it in comments how to invoke gdc (which we didn't package) and rdmd (rdmd source itself doesn't mention pkg-config either) - both are in shell expressions like gdc CoreGL.d `pkg-config gtkd-3 gl --cflags --libs` and rdmd `pkg-config gtkd-3 --cflags` -L-lGL -L-ldl CoreGL.d). No non-comment reference at all.

That said, we could add pkg-config as an implicit input so that if D packages decided to directly use it in the future they'd pick up the same one.

> Or we could simply let people add pkg-config as an input when it’s
> necessary, just like we do for ‘gnu-build-system’ packages.

dub itself does automatically use pkg-config.
It's as if make always used pkg-config (whether you write "pkg-config" into a Makefile or not).

Also, if pkg-config is not available dub will silently fallback to guessing. It will not fail (and that's bad!).

I've just had to fix a problem (in another non-D package) where it didn't use pkg-config and so it didn't pick up the Unicode codepoint "-D" option and it broke some stuff silently at runtime. That's exactly what can happen when programs guess the library import flags.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] gnu: dub: Patch pkg-config name.
  2017-02-03 17:38       ` Danny Milosavljevic
@ 2017-02-07 14:13         ` Ludovic Courtès
  0 siblings, 0 replies; 7+ messages in thread
From: Ludovic Courtès @ 2017-02-07 14:13 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: guix-devel

Hi Danny,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> On Fri, 03 Feb 2017 17:49:38 +0100
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> I think ‘dub-build-system’ could add it as an implicit input, much like
>> ‘gnu-build-system’ adds binutils as an implicit input.
>
> Okay, but it's directly used only by dub (it its function of building D packages).
>
> I don't think D packages themselves even know what pkg-config is.
>
> The ldc 1.1.0 sources don't even mention "pkg-config" once - neither do any of the D packages I tried except gtk-d. That one mentions it in comments how to invoke gdc (which we didn't package) and rdmd (rdmd source itself doesn't mention pkg-config either) - both are in shell expressions like gdc CoreGL.d `pkg-config gtkd-3 gl --cflags --libs` and rdmd `pkg-config gtkd-3 --cflags` -L-lGL -L-ldl CoreGL.d). No non-comment reference at all.

OK.

> That said, we could add pkg-config as an implicit input so that if D packages decided to directly use it in the future they'd pick up the same one.

Yeah.

>> Or we could simply let people add pkg-config as an input when it’s
>> necessary, just like we do for ‘gnu-build-system’ packages.
>
> dub itself does automatically use pkg-config.
> It's as if make always used pkg-config (whether you write "pkg-config" into a Makefile or not).
>
> Also, if pkg-config is not available dub will silently fallback to guessing. It will not fail (and that's bad!).

OK.

Well I think the conclusion is that (1) we really need to make sure DUB
has pkg-config around one way or another, and (2) of all the solutions
we discussed, I don’t see one that is significantly “better” than the
other (it was worth discussing them though, because in many cases
there’s a solution that “looks better”).

So I’d say proceed as you prefer.

Thanks for taking the time to explain!

Ludo’.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-02-07 14:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-31 16:00 [PATCH] gnu: dub: Patch pkg-config name Danny Milosavljevic
2017-02-01 22:22 ` Ludovic Courtès
2017-02-02 19:36   ` Danny Milosavljevic
2017-02-03 15:14     ` Danny Milosavljevic
2017-02-03 16:49     ` Ludovic Courtès
2017-02-03 17:38       ` Danny Milosavljevic
2017-02-07 14:13         ` Ludovic Courtès

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.