mbox series

[bug#54088,0/2] julia-build-system: Add missing julia-pkg-deps

Message ID cover.1645447521.git.efraim@flashner.co.il
Headers
Series julia-build-system: Add missing julia-pkg-deps |

Message

Efraim Flashner Feb. 21, 2022, 12:47 p.m. UTC 
It turns out we didn't expose julia-package-dependencies in the
julia-build-system, making the created Package.toml incomplete. This
corrects the omission and fixes the one package where it is manually
added.

Efraim Flashner (2):
  build: julia: Add julia-package-dependencies as keyword.
  gnu: julia-media: Use julia-package-dependencies.

 gnu/packages/julia-xyz.scm        | 15 ++-------------
 guix/build-system/julia.scm       |  5 ++++-
 guix/build/julia-build-system.scm |  7 ++++++-
 3 files changed, 12 insertions(+), 15 deletions(-)


base-commit: 7eefff2054b94f8a7ad850ad8f36b8773bb39ce9

Comments

Simon Tournier Feb. 23, 2022, 1:44 p.m. UTC | #1 
Hi Efraim,

On Mon, 21 Feb 2022 at 14:47, Efraim Flashner <efraim@flashner.co.il> wrote:

> It turns out we didn't expose julia-package-dependencies in the
> julia-build-system, making the created Package.toml incomplete. This
> corrects the omission and fixes the one package where it is manually
> added.

All LGTM.  But I take this opportunity to uniform, as discussed:

        It's definitely the new pattern we're using now. I suppose it's
        not really necessary here, but having more instances of it
        throughout the codebase also makes it easier to search for
        examples when others are looking to use or modify gexps.

        <https://issues.guix.gnu.org/issue/53656#4-lineno49>

The first patch of the series uses G-exps.  Note that it does not imply
any rebuild.

The Julia build system is adapted for the new
'julia-package-dependencies' and so it requires the minor tweak to
default with ''() instead of #f; otherwise it crashes for some packages.

Since we are at it, I fix a minor typo of how Julia uses --procs.  Other
said, currently "guix build julia-media -c 0" works but not "guix build
julia-media -c 1" which crashes.

Because the build system modifications imply a full Julia rebuild, let
take the opportunity to remove the trailing #t.  It is done package per
package because each modified package need a rebuild so it eases to
follow, IMHO.  They can be squashed otherwise.


Cheers,
simon
Efraim Flashner Feb. 24, 2022, 10:24 a.m. UTC | #2 
Great job! Patches pushed (with one change)

On Wed, Feb 23, 2022 at 02:44:43PM +0100, zimoun wrote:
> Hi Efraim,
> 
> On Mon, 21 Feb 2022 at 14:47, Efraim Flashner <efraim@flashner.co.il> wrote:
> 
> > It turns out we didn't expose julia-package-dependencies in the
> > julia-build-system, making the created Package.toml incomplete. This
> > corrects the omission and fixes the one package where it is manually
> > added.
> 
> All LGTM.  But I take this opportunity to uniform, as discussed:
> 
>         It's definitely the new pattern we're using now. I suppose it's
>         not really necessary here, but having more instances of it
>         throughout the codebase also makes it easier to search for
>         examples when others are looking to use or modify gexps.
> 
>         <https://issues.guix.gnu.org/issue/53656#4-lineno49>
> 
> The first patch of the series uses G-exps.  Note that it does not imply
> any rebuild.
> 
> The Julia build system is adapted for the new
> 'julia-package-dependencies' and so it requires the minor tweak to
> default with ''() instead of #f; otherwise it crashes for some packages.
> 
> Since we are at it, I fix a minor typo of how Julia uses --procs.  Other
> said, currently "guix build julia-media -c 0" works but not "guix build
> julia-media -c 1" which crashes.
> 
> Because the build system modifications imply a full Julia rebuild, let
> take the opportunity to remove the trailing #t.  It is done package per
> package because each modified package need a rebuild so it eases to
> follow, IMHO.  They can be squashed otherwise.
> 
> 
> Cheers,
> simon
Simon Tournier Feb. 24, 2022, 10:39 a.m. UTC | #3 
On Thu, 24 Feb 2022 at 12:24, Efraim Flashner <efraim@flashner.co.il> wrote:

> Great job! Patches pushed (with one change)

Thanks!

Cheers,
simon