Remarks about commit 2c82d4ad10de8

Remarks about commit 2c82d4ad10de8

[Mathieu Othacehe]

Hello Brett,

I have a few remarks on the aforementioned commit.

> +       (list (string-append "-DCMAKE_CXX_FLAGS='-isystem "
> +                            (assoc-ref %build-inputs "gcc")
> +                            "/include/c++'"))))

Why is this needed? The following snippet in clang-from-llvm isn't enough?

--8<---------------cut here---------------start------------->8---
;; Make clang look for libstdc++ in the right
;; location.
(("LibStdCXXIncludePathCandidates\\[\\] = \\{")
 (string-append
  "LibStdCXXIncludePathCandidates[] = { \"" gcc "/include/c++\","))
--8<---------------cut here---------------end--------------->8---

> +     (inputs
>       `(("clang" ,clang)
> -       ("llvm" ,llvm)))
> +       ("ncurses" ,ncurses)))
> +    (native-inputs
> +     `(("rapidjson" ,rapidjson)

Rapidjson is an input (even if ccls is not linked against because its a
header-only library).

> +       ("gcc" ,gcc)))

This shouldn't be needed as it is an implicit input.

Thanks,

Mathieu

Re: Remarks about commit 2c82d4ad10de8

[Brett Gilio]

I think you are correct. I likely made a mistake. We should revert the change.

Thanks!

Dec 16, 2019 9:13:31 AM Mathieu Othacehe <m.othacehe@gmail.com>:

> 
> Hello Brett,
> 
> I have a few remarks on the aforementioned commit.
> 
> 
> > + (list (string-append "-DCMAKE_CXX_FLAGS='-isystem "
> > + (assoc-ref %build-inputs "gcc")
> > + "/include/c++'"))))
> > 
> 
> Why is this needed? The following snippet in clang-from-llvm isn't enough?
> 
> --8<---------------cut here---------------start------------->8---
> ;; Make clang look for libstdc++ in the right
> ;; location.
> (("LibStdCXXIncludePathCandidates\\[\\] = \\{")
> (string-append
> "LibStdCXXIncludePathCandidates[] = { \"" gcc "/include/c++\","))
> --8<---------------cut here---------------end--------------->8---
> 
> 
> > + (inputs
> > `(("clang" ,clang)
> > - ("llvm" ,llvm)))
> > + ("ncurses" ,ncurses)))
> > + (native-inputs
> > + `(("rapidjson" ,rapidjson)
> > 
> 
> Rapidjson is an input (even if ccls is not linked against because its a
> header-only library).
> 
> 
> > + ("gcc" ,gcc)))
> > 
> 
> This shouldn't be needed as it is an implicit input.
> 
> Thanks,
> 
> Mathieu
> 

Re: Remarks about commit 2c82d4ad10de8

[Brett Gilio]

Brett Gilio <brettg@posteo.net> writes:

> I think you are correct. I likely made a mistake. We should revert the change.
>
> Thanks!
>
> Dec 16, 2019 9:13:31 AM Mathieu Othacehe <m.othacehe@gmail.com>:
>
>> 
>> Hello Brett,
>> 
>> I have a few remarks on the aforementioned commit.
>> 
>> 
>> > + (list (string-append "-DCMAKE_CXX_FLAGS='-isystem "
>> > + (assoc-ref %build-inputs "gcc")
>> > + "/include/c++'"))))
>> > 
>> 
>> Why is this needed? The following snippet in clang-from-llvm isn't enough?
>> 
>> --8<---------------cut here---------------start------------->8---
>> ;; Make clang look for libstdc++ in the right
>> ;; location.
>> (("LibStdCXXIncludePathCandidates\\[\\] = \\{")
>> (string-append
>> "LibStdCXXIncludePathCandidates[] = { \"" gcc "/include/c++\","))
>> --8<---------------cut here---------------end--------------->8---
>> 
>> 
>> > + (inputs
>> > `(("clang" ,clang)
>> > - ("llvm" ,llvm)))
>> > + ("ncurses" ,ncurses)))
>> > + (native-inputs
>> > + `(("rapidjson" ,rapidjson)
>> > 
>> 
>> Rapidjson is an input (even if ccls is not linked against because its a
>> header-only library).
>> 
>> 
>> > + ("gcc" ,gcc)))
>> > 
>> 
>> This shouldn't be needed as it is an implicit input.
>> 
>> Thanks,
>> 
>> Mathieu
>> 
>
>

Mathieu,

I have reverted the affected commits. Thank you for bringing the issue
to my attention.

-- 
Brett M. Gilio
GNU Guix, Contributor <https://guix.gnu.org/>