Subject: Re: [PATCH 0/1] Add C3

From: Adam Faiz <[email-masked]>
Date: Tue, 06 Aug 2024 15:07:07 +0000

Hello Ashvith,

I'd like to give my feedback on this patch series. I'll send an updated patch series using the suggestions below if I have the time.

> +(define-public c3c-bootstrap
> +  (package
> +    (name "c3c")
> +    (version "0.6.1")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/c3lang/c3c")
> +             (commit (string-append "v" version))))
> +       (file-name (git-file-name name version))
> +       (sha256
> +        (base32 "0xjl3yghyqmqv9118nhcix3xywlpdirqc0dlmdaghdkja0wr19rw"))
> +       (snippet #~(begin
> +                    (use-modules (guix build utils))
> +                    (substitute* "CMakeLists.txt"
> +                      (("\\$\\{LLVM_LIBRARY_DIRS\\}")
> +                       (string-append #$lld-18 "/lib/ "
> +                                      #$llvm-18 "/lib/")))))))
> +    (build-system cmake-build-system)
> +    (arguments
> +     '(#:configure-flags '("-DCMAKE_INSTALL_LIBDIR=lib")
> +       ;; TODO: Fix test
> +       #:tests? #f))

According to its README, the unit tests are run using the built c3c with its compile-test subcommand: `c3c compile-test test/unit`. The `check` build phase needs to be replaced to run this command.

A patch sent upstream to add a proper `check` target would be good, and the test/ subdirectory would probably also need to be specified as part of the out-of-tree build.

> +    (inputs (list curl libffi llvm-18 lld-18))

The curl dependency should be removed, since c3c's optional dependency downloading functionality isn't needed in Guix.