On 2025-04-25 10:57, Ludovic Courtès wrote:

> Hello,
>
> Nicolas Graves <ngra...@ngraves.fr> writes:
>
>> Actually the thunk was not necessary because args were already passed to
>> the build-bag procedure, and modules and imported-modules were already
>> used in every bag-build procedures, except for trivial and raw
>> build-systems.
>>
>> Patch should look like the one attached, overall pretty simple.

I'll probably additionally drop defaults in bag-build procedures, to
make it clear that the defaults are coming from <build-system>-modules
rather than resulting from default function argument.

>> However it does break the build-system API so channels that define a
>> build-system will have to update too.
>
> Maybe we the field could default to the empty list?

Yes, I was unsure what to do there.

The issue is that for dependent channels, it's probably better to have a
clear failure (e.g. "missing required #:imported-modules") rather than
having '() silently passed in modules and imported-modules.

There are only raw, trivial and channel that use '(), so IMO not a clear
requirement. I guess it depends on which error is clearer for dependent
channels.  IMHO, it might be better to keep no defaults, as to not
silently override current dependent channels modules/imported-modules,
and making them fail with a clear error message instead.

I could use #:allow-other-keys instead of adding unused
(imported-)modules keys to functions, if it makes it clearer.

WDYT ?

>
>> I'll investigate now if we can simply do away without imported-modules
>> at all.
>
> Yes, we should definitely do that.

Actually I think the 68315 would be a requirement to do that properly
(brainstorming here).

Let's say we want some arbitrary way to extend imported modules on the
build-system level (i.e. not the package level).  We already explored
two solutions :
- lower procedure properties
- build-system field

If we remove build-system field, I don't know how I can do the generic
build-system transformation in my use-case.

But with 68315 the returned value from the bag-build is a monadic gexp (rather
than a monadic derivation), I probably can do something like: 

(define (make-new-lower old-lower other-args)
  (lambda* args
    (let ((old-bag (apply old-lower args)))
      (bag
        (inherit old-bag)
        (build
         (lambda* (name inputs #:key (outputs '("out"))
                        #:allow-other-keys #:rest rest)
           (mlet %store-monad
               ((builder (apply (bag-build old-bag)
                                name inputs #:outputs outputs rest)))
             (return
              (with-imported-modules additional-imported-modules
                #~(begin
                  (use-modules ,@additional-modules)
                  (more-code)
                  (any-wrapper 
                    #$builder))))))))))

which is my use-case (and the reason why I needed those in the first
place).

I don't know how to do that kind of thing if we only drop the
default-imported-modules field without 68315.  Am I missing something
here?

(The reason I need that is that I want guix local instantiate (see next
answer) to support a --git option and not drop the .git directory, but
still apply guix package-source patches/snippet.  IIUC, I some
additional code thus some additional imported-modules to support that.)

>> +  (default-modules          build-system-default-modules)
>
> … and probably change this one to just ‘modules’.

OK.

>> 1) some patches are improvements independent of wherever I try to do with
>> partial builds.  They can already be reviewed now independently from the 
>> rest.
>
> “Partial build”, interesting.  :-)

I actually have something working pretty well for trivial packages
without patches nor snippets.  Workflow is, in any arbitrary directory:

$ guix local instantiate hello
$ cd hello* (I have to figure the proper chdir after gnu:unpack, and do
that in the previous call guix local instantiate).
$ guix local build hello

That would build hello locally (in said dir, and install each output in
./$output dir, with all guix-rich info: arguments, store-injected
shebangs etc), and keep compilation caches.  There an additional caveat:
I drop the 'install-license-files phase, because it is likely dependent
on having the output in the store for some reason.

>> 2) some patches are standardizing improvements for the names :
>>   - %XXX-build-system-modules --> %default-XXX-imported-modules
>>   - %XXX-modules or %default-modules --> %default-XXX-modules
>>   - is that a fine standardizing change or should I go through a GCD?
>
> Well, there was already a name change in this area that left me
> unconvinced and that we’re still adjusting to, many months later.  So my
> advice would be to think twice before renaming bindings with a lot of
> users.  A GCD might sound overkill but OTOH it could help think through
> the implications.

I'm not really that much into it either, just thought some
standardization is welcome based on the current state of build-systems.
I'll see if it's easy to separate patches where I create the
%default-XXX-modules (rarely used but still cleaner in the file) to be
applicable before the %default-XXX-imported-modules ones, which would
probably require a GCD, and are not a requirement in my use-case.

>> 3) some patches are about the core of 68315 (allowing monadic bag-build).

Read "monadic gexp" here.

-- 
Best regards,
Nicolas Graves

Reply via email to