[bug#71897] [PATCH v8 6/8] gnu: Add cpp-ada-url-parser.

[Maxim Cournoyer]

Hi Dan,

dan <i@dan.games> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> That's not upstreamable work, so should ideally be marked as such as
>> a
>> patch at the top of the patch file (Upstream-status: N/A or
>> similar).
>> It's a bit sad that this will need ongoing maintenance (careful
>> rebasing
>> when they touch that file).  I'd like to see the problem with CPM
>> commented in a bit more details -- wondering if it could be made to
>> work
>> without patching it out, e.g. perhaps some CMake variables can be
>> set to
>> have CPM work offline?
>
> In the previous iteration, I disabled ADA_TESTING and ADA_TOOLS so
> that no patches are needed.  However, Liliana suggest that "we should
> enable testing, even if we need to patch the cmake files to unvendor
> inputs".  I think we have to make a decision here: we either accept a
> package with testing disabled, or we need extra work to maintain the
> patch in the future.

I agree with Liliana.  We strive to enable test suites in Guix packages,
as this gives us a much easier time detecting breakages early when
upgrading dependent packages.

My suggestion was to look a little bit deeper in the problem at hand in
case to see if it can be configured to work for our needs.  If it can't,
then your approach is perfectly fine (ideally with a feature request
upstream requesting an easier way to run their test suite offline); if
it can, we should configure it instead and avoid having to maintain
extra patches.

> I took a brief look at CPM, and it seems possible to fetch
> dependencies from local directories, but I assume even we go this way
> we still need to patch their CMakeLists.txt files.

There's a relatively new FetchContent module in CMake that perhaps CPM
abstracts?  If it's using FetchContent, it should be possible to have
the build system attempt to use a system-provided library instead of
fetching from a submodule and building it from source.  See the 'jami'
package, for example, which sets
'-DFETCHCONTENT_TRY_FIND_PACKAGE_MODE=ALWAYS' to have FetchContent do
that.

-- 
Thanks,
Maxim