Hi Danny, Danny Milosavljevic <dan...@scratchpost.org> skribis:
> * guix/build-system/dub.scm: New file. > * guix/build/dub-build-system.scm: New file. > * Makefile.am (MODULES): Add them. Nice work! Do you have experience using it on real DUB packages? IOW, how complete is it? :-) For the final patch, could you add a short description under “Build Systems” in guix.texi? > +(define* (dub-build store name inputs > + #:key > + (tests? #t) > + (test-target #f) > + (configure-flags #f) ; XXX unused You can remove keyword parameters that don’t make sense like this one. > +(define-module (guix build dub-build-system) > + #:use-module ((guix build gnu-build-system) #:prefix gnu:) > + #:use-module (guix build syscalls) > + #:use-module (guix build utils) > + #:use-module (ice-9 popen) > + #:use-module (ice-9 rdelim) > + #:use-module (ice-9 ftw) > + #:use-module (ice-9 format) > + #:use-module (ice-9 match) > + #:use-module (srfi srfi-1) > + #:use-module (srfi srfi-26) > + #:export (%standard-phases > + dub-build)) > + > +;; Commentary: > +;; > +;; Builder-side code of the standard Rust package build procedure. s/Rust/DUB/ :-) Maybe write “DUB, the build tool for D” (or similar) to clarify. > +;; FIXME: Needs to be parsed from url not package name. > +(define (package-name->d-package-name name) > + "Return the package name of NAME." “Return the DUB package name corresponding to NAME, a Guix package name.” (Correct?) > +(define* (configure #:key inputs #:allow-other-keys) > + "Replace argo.toml [dependencies] section with guix inputs." Maybe “Add INPUTS to the 'dependencies' section of 'argo.toml'.”? > + (system* "chmod" "+w" ".") Rather: (chmod "." #o755). > + (mkdir "vendor") “vendor”? > + (for-each > + (match-lambda > + ((name . path) > + (let* ((d-package (package-name->d-package-name name)) > + (d-basename (basename path))) > + (when (and d-package path) > + (match (string-split (basename path) #\-) > + ((_ ... version) > + (symlink (string-append path "/lib/dub/" d-basename) > (string-append "vendor/" d-basename)))))))) > + inputs) Could you add a comment above explaining why this needs to be done? Please keep lines below 80 chars. :-) > + ;(setenv "CC" (string-append (assoc-ref inputs "gcc") "/bin/gcc")) Remove. > + (let* ((dir (mkdtemp! "/tmp/dub.XXXXXX"))) > + (setenv "HOME" dir) If HOME is relied on, please add a comment explaining why. > +(define* (build #:key (dub-build-flags '()) > + #:allow-other-keys) > + "Build a given DUB package." > + (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"))) > + #t 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)) (These files are probably small so it should be good enough.) > + (let ((status (zero? (apply system* `("dub" "build" > ,@dub-build-flags))))) > + (system* "dub" "run") ; might fail for "targetType": "library" > + status))) Should be (and (zero? (apply system* "dub" "build" dub-build-flags)) (zero? (apply system* "dub" "run"))) Or is it important to ignore the exit status of “dub run”? > +(define* (check #:key tests? #:allow-other-keys) > + (if tests? > + (zero? (system* "dub" "test")) > + #t)) To be sure, could you pass the file through etc/indent-code.el? > +(define* (install #:key inputs outputs #:allow-other-keys) > + "Install a given DUB package." > + (let* ((out (assoc-ref outputs "out")) > + (outbin (string-append out "/bin")) > + (outlib (string-append out "/lib/dub/" (basename out)))) In other places these variables are just called “bin” and “lib”. > + (mkdir-p outbin) > + ;; TODO remove "-test-application" > + (copy-recursively "bin" outbin) > + (mkdir-p outlib) > + (delete-file-recursively "vendor") ; contains timestamps > + (copy-recursively "." (string-append outlib)) (string-append outlib) -> outlib. > +(define* (dub-build #:key inputs (phases %standard-phases) > + #:allow-other-keys #:rest args) > + "Build the given DUB package, applying all of PHASES in order." > + (apply gnu:gnu-build #:inputs inputs #:phases phases args)) You can remove this one since it’s unused. Thank you! Ludo’.