Re: [PATCH v3 2/2] Makefile: use $(OBJECTS) instead of $(C_OBJ)

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 1, 2022, 15:48 UTC
Message-ID: <xmqqsflbayeh.fsf@gitster.g>
In-Reply-To: <220901.867d2njg52.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 17 quoted lines

>> This is a declaration that we would never ever build .o files out of
>> sources other than .c files.  While it does make sense to have it
>> outside the scope of [PATCH 1/2], I am not sure if it even belongs
>> to the same series.
>
> I think it does. Before this the C_OBJ would be:
>
> 	C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
>
> but after 1/2 it's the same as $(OBJECTS). An earlier iteration of this
> did this cleanup "while we're at it" (which I do think makes sense as an
> atomic change), but I got the feedback that the cleanup wasn't strictly
> necessary.
>
> But as 1/2 has removed the ability to build those $(ASM_OBJ), as we had
> only one of those, I don't think keeping this particular bit of
> indirection makes sense.

You are not thinking for longer term to help project maintenance.

This change removes distinction between C_OBJ and OBJECTS, only because the sources to the objects we HAPPEN TO have are only C files. It is premature and short sighted to declare that it has to stay that way forever. And such a declaration is not something we would casually make "while at it" in a topic like this.

When we add a source written in another language, say xyzzy, to be compiled into an object file, we'd add $(XYZZY_OBJ), and they will become part of $(OBJECTS), but the current rule to create $(C_OBJ) will not apply to $(XYZZY_OBJ). But you do this:

    -$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
    +$(OBJECTS): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
            $(QUIET_CC)$(CC) -o $*.o -c ... $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<

Right now, we know where this patch affected the build procedure, because the patch highlights what is being changed. But when future developers need to produce some files that belong to $(OBJECTS) out of source files that are not .c, they first need to locate the above hunk and revert it. I do not see the benefit of being hostile to future developers with this patch. Not before we know that it is not likely that we would add any non-C sources in the future, by running with 1/2 alone for a year or two.