Re: [PATCH] Makefile: pin cargo --target-dir to ./target

[Junio C Hamano <gitster@pobox.com>]

Andy Koppe <andy.koppe@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Andy Koppe <andy.koppe@gmail.com> writes:
>>
>> > The problem I see with that approach is that CARGO_ARGS looks like it
>> > is intended for build preferences similar to CFLAGS or LDFLAGS that
>> > could be overridden on the make command line, whereas the target dir
>> > is a hard requirement of the subsequent link step.
>> >
>> > I suppose `override CARGO_ARGS += --target-dir target` would avoid
>> > that concern, but also set a trap because any subsequent modifications
>> > of the variable without `override` would be quietly ignored.
>>
>> But we are already tweaking it with += in the Makefile, aren't we?
>>
>>     # Rust flags
>>     CARGO_ARGS =
>>     ifndef V
>>     CARGO_ARGS += --quiet
>>     endif
>>     ifndef DEBUG
>>     CARGO_ARGS += --release
>>     endif
>
> Yes, but those are preferences that can be overridden on the make
> command line without doing harm (within reason), similar to the
> default 'CFLAGS = -g -O2 -Wall', whereas the --target-dir should
> always be passed to ensure the cargo output is going where it's
> expected.

That's fair.

So if we wanted to go that route, we'd need a companion variable for
CARGO_ARGS that is similar to BASIC_CFLAGS we have for CFLAGS, which
may not be a bad thing to have in the longer term assuming that we
would have more targets that need to use CARGO_ARGS.

Anyway, this was not an objection to the way the patch was
originally written.  Just I noticed while resolving conflicts
between in-flight topics.

Thanks.