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

From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, "Mike Hommey" <mh@glandium.org>,
	"brian m . carlson" <sandals@crustytoothpaste.net>,
	"Carlo Marcelo Arenas Belón" <carenas@gmail.com>,
	"Taylor Blau" <me@ttaylorr.com>
Subject: Re: [PATCH v3 2/2] Makefile: use $(OBJECTS) instead of $(C_OBJ)
Date: Thu, 01 Sep 2022 08:48:38 -0700	[thread overview]
Message-ID: <xmqqsflbayeh.fsf@gitster.g> (raw)
In-Reply-To: <220901.867d2njg52.gmgdl@evledraar.gmail.com> (=?utf-8?B?IsOG?= =?utf-8?B?dmFyIEFybmZqw7Zyw7A=?= Bjarmason"'s message of "Thu, 01 Sep 2022 16:52:05 +0200")

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

>> 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.