Re: [PATCH v2] gnu: Add dub-build-system.

[Ludovic Courtès]

Hello!

Danny Milosavljevic <address@hidden> skribis:

>> Do you have experience using it on real DUB packages?  IOW, how complete
>> is it?  :-)
>
> Yeah, I've tested it on a small subset of all code.dlang.org packages. About 
> half of the ones I tested work fine. This is my log:

Impressive!  I guess we can consider the importer mostly ready.  :-)

>>Rather: (chmod "." #o755).
>
> Thanks. Note that it's a workaround because git-download leaves the build 
> directory read-only for some reason. Should the problem be found and fixed 
> there? Or is it on purpose?

It’s not really on purpose, it’s just that the checkout in /gnu/store is
read-only and the ‘unpack’ phase simply copies it as-is.

>>Could you add a comment above explaining why this needs to be done?
>
> Sure. (it prepares a directory which can be used to find all the (D) 
> dependencies by going just one level down; earlier versions just used 
> /gnu/store directly but that would mean if you put such a package (without 
> any subdirs in it) into your profile it would pollute the root)
>
> I'm just thinking out loud about the comment :)
>
> Something like this?
>
> ;; Prepare one new directory with all the required dependencies.
> ;; It's necessary to do this (instead of just using /gnu/store as the 
> directory) because we want to hide the libraries in subdirectories 
> lib/dub/... instead of polluting the user's profile root.

Sounds good (with wrapped lines).

>>If HOME is relied on, please add a comment explaining why.
>
> The dub build system uses it to [...]

No need to comment here, I trust you to do the right thing ;-), but
please just provide an explanation as a comment so people later can know
why things are done this way.

>> +  (if (or (zero? (system* "grep" "-q" "sourceLibrary" "package.json"))
>> +          (zero? (system* "grep" "-q" "sourceLibrary" "dub.sdl")) ; note: 
>> format is different!
>> +          (zero? (system* "grep" "-q" "sourceLibrary" "dub.json")))
>
>>Would be best to avoid calling out to ‘grep’.  At worst you can do:
>>  (define (grep string file)
>>    (string-contains (call-with-input-file file get-string-all)
>>                     string))
>
> Thanks. I'll use it. Will it break when it can't find the file? That
> would be bad. Usually there's only one of these files available.

Then you can do, say:

  (or (not (file-exists? file))
      (string-contains …))

With these issues addressed, I think you can push the updated patch.

Thank you!

Ludo’.