archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <>
To: "Ævar Arnfjörð Bjarmason" <>
Cc:, "Mike Hommey" <>,
	"brian m . carlson" <>,
	"Carlo Marcelo Arenas Belón" <>,
	"Taylor Blau" <>
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: <> (=?utf-8?B?IsOG?= =?utf-8?B?dmFyIEFybmZqw7Zyw7A=?= Bjarmason"'s message of "Thu, 01 Sep 2022 16:52:05 +0200")

Ævar Arnfjörð Bjarmason <> 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.

      reply	other threads:[~2022-09-01 15:48 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-12  8:56 SHA1dc on mac Mike Hommey
2020-02-12 16:46 ` Eric Sunshine
2020-02-12 22:31   ` Mike Hommey
2020-02-12 22:40     ` Junio C Hamano
2020-02-23 22:37       ` [PATCH] Remove non-SHA1dc sha1 implementations Mike Hommey
2020-02-24  4:47         ` Jeff King
2020-02-24  4:52           ` Jeff King
2022-03-19  1:02         ` [PATCH] ppc: remove custom SHA-1 implementation Ævar Arnfjörð Bjarmason
2022-03-21 16:39           ` Junio C Hamano
2022-03-21 17:06           ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2022-03-21 21:19             ` brian m. carlson
2022-08-31  9:18             ` [PATCH v3 0/2] Makefile + hash.h: remove PPC_SHA1 implementation Ævar Arnfjörð Bjarmason
2022-08-31  9:18               ` [PATCH v3 1/2] " Ævar Arnfjörð Bjarmason
2022-08-31  9:18               ` [PATCH v3 2/2] Makefile: use $(OBJECTS) instead of $(C_OBJ) Ævar Arnfjörð Bjarmason
2022-08-31 21:44                 ` Junio C Hamano
2022-09-01 14:52                   ` Ævar Arnfjörð Bjarmason
2022-09-01 15:48                     ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqsflbayeh.fsf@gitster.g \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).