xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Anthony PERARD <anthony.perard@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Ian Jackson <iwj@xenproject.org>, Julien Grall <julien@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [XEN PATCH v6 07/31] build,include: rework compat-build-source.py
Date: Tue, 13 Jul 2021 09:37:00 +0200	[thread overview]
Message-ID: <3ec5fa48-9079-3e8b-f5d7-41c2b1b923f1@suse.com> (raw)
In-Reply-To: <YOxTKHCQDSs2mHG8@perard>

On 12.07.2021 16:35, Anthony PERARD wrote:
> On Wed, Jul 07, 2021 at 04:58:29PM +0200, Jan Beulich wrote:
>> On 01.07.2021 16:09, Anthony PERARD wrote:
>>> Improvement are:
>>> - give the path to xlat.lst as argument
>>> - include `grep -v` in compat-build-source.py script, we don't need to
>>>   write this in several scripted language.
>>>
>>> Also remove dependency on Makefile as the file generation doesn't
>>> depend on it anymore.
>>
>> Did it before any more?
> 
> Kind of, yes, there is "grep -v" that makes the Makefile part of the
> script that generates the target.
> 
>> In the subsequent patch I can see more of
>> a reason to drop the dependency, but neither there nor here I'm
>> really convinced: In general I think every generate file would
>> better depend on the makefile containing the rule used for its
>> building, as a change to that rule means the target wants
>> rebuilding.
> 
> Does that mean that nearly every single targets should depends on a
> "Makefile" or on "Rules.mk" ? :-)

Well, yes, I indeed think so.

> As for the current target "compat/%.c", with this patch applied, the
> only few things that the content of the file depends on is the script,
> the first dependency, and "xlat.lst" (also a dependency). So the
> Makefile doesn't play a role into what get's into the target, the
> "mkdir" and the "mv" don't really matter. If the rule where to be
> changed in a way that changed the content of the target, that would be
> wrong in my opinion, the change should be done in the script.
> If someone wanted to rewrite the script in a different language and thus
> renaming the script, that would be fine too as make would notice that
> the new script is newer that the target (as the file for the script as
> just been created).

Right, but e.g. adding an option to the scrip invocation would then
not trigger a rebuild.

> But, I guess we could start to use the "if_changed" macro here to
> detected rule changes.
> 
> I really don't like when a target depends on a "Makefile" because that
> means the target gets rebuilt for unrelated reason so I'd like to avoid
> dependency on it when possible.

I can see the background of this viewpoint, but personally I think the
extra rebuilds are the price to pay to not have any rebuilds skipped
when they would be needed. Overall it is my understanding that to get
this more fine grained is what if_changed is for.

Jan



  reply	other threads:[~2021-07-13  7:37 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-01 14:09 [XEN PATCH v6 00/31] xen: Build system improvements Anthony PERARD
2021-07-01 14:09 ` [XEN PATCH v6 01/31] build: fix %.s: %.S rule Anthony PERARD
2021-07-05 14:40   ` Jan Beulich
2021-07-01 14:09 ` [XEN PATCH v6 02/31] build: introduce cpp_flags macro Anthony PERARD
2021-07-07 14:18   ` Jan Beulich
2021-07-12 10:53     ` Anthony PERARD
2021-07-01 14:09 ` [XEN PATCH v6 03/31] build: use if_changed on built_in.o Anthony PERARD
2021-07-08 12:03   ` Andrew Cooper
2021-07-12 11:08     ` Anthony PERARD
2021-07-01 14:09 ` [XEN PATCH v6 04/31] build: use if_changed_rules with %.o:%.c targets Anthony PERARD
2021-07-01 14:09 ` [XEN PATCH v6 05/31] build: factorise generation of the linker scripts Anthony PERARD
2021-07-07 14:25   ` Jan Beulich
2021-07-12 11:02     ` Anthony PERARD
2021-07-13  7:28       ` Jan Beulich
2021-07-01 14:09 ` [XEN PATCH v6 06/31] x86/mm: avoid building multiple .o from a single .c file Anthony PERARD
2021-07-07 14:45   ` Jan Beulich
2021-07-12 12:49     ` Anthony PERARD
2021-07-13  7:32       ` Jan Beulich
2021-07-01 14:09 ` [XEN PATCH v6 07/31] build,include: rework compat-build-source.py Anthony PERARD
2021-07-07 14:58   ` Jan Beulich
2021-07-12 14:35     ` Anthony PERARD
2021-07-13  7:37       ` Jan Beulich [this message]
2021-07-01 14:09 ` [XEN PATCH v6 08/31] build,include: rework compat-build-header.py Anthony PERARD
2021-07-01 14:09 ` [XEN PATCH v6 09/31] build: clean "lib.a" Anthony PERARD
2021-07-07 15:03   ` Jan Beulich
2021-07-12 14:42     ` Anthony PERARD
2021-07-01 14:09 ` [XEN PATCH v6 10/31] build: use $(kconfig) shortcut in clean rule Anthony PERARD
2021-07-07 15:05   ` Jan Beulich
2021-07-01 14:09 ` [XEN PATCH v6 11/31] build: fix clean targets when subdir-y is used Anthony PERARD
2021-07-07 15:15   ` Jan Beulich
2021-07-12 14:54     ` Anthony PERARD
2021-07-01 14:09 ` [XEN PATCH v6 12/31] build: use subdir-y in test/Makefile Anthony PERARD
2021-07-07 15:26   ` Jan Beulich
2021-07-12 15:22     ` Anthony PERARD
2021-07-13  7:41       ` Jan Beulich
2021-07-13 14:35         ` Anthony PERARD
2021-07-01 14:09 ` [XEN PATCH v6 13/31] build,tools: have default rules depends on symbols Anthony PERARD
2021-07-07 15:28   ` Jan Beulich
2021-07-01 14:09 ` [XEN PATCH v6 14/31] build,arm: move LDFLAGS change to arch.mk Anthony PERARD
2021-07-01 14:09 ` [XEN PATCH v6 15/31] build: move make option changes check earlier Anthony PERARD
2021-07-07 15:40   ` Jan Beulich
2021-07-12 16:21     ` Anthony PERARD
2021-07-13  7:42       ` Jan Beulich
2021-07-01 14:09 ` [XEN PATCH v6 16/31] build: avoid building arm/arm/*/head.o twice Anthony PERARD
2021-07-01 14:09 ` [XEN PATCH v6 17/31] build: convert binfile use to if_changed Anthony PERARD
2021-07-07 15:48   ` Jan Beulich
2021-07-12 16:32     ` Anthony PERARD
2021-07-13  7:51       ` Jan Beulich
2021-07-13 14:58         ` Anthony PERARD
2021-07-13 15:33           ` Jan Beulich
2021-07-01 14:09 ` [XEN PATCH v6 18/31] xen: move include/asm-* to include/arch-*/asm Anthony PERARD
2021-07-01 17:24   ` Paul Durrant
2021-07-01 17:26   ` Bob Eshleman
2021-08-05  7:04   ` Jan Beulich
2021-08-09 13:20     ` Anthony PERARD
2021-07-01 14:09 ` [XEN PATCH v6 19/31] build: rework .banner generation Anthony PERARD
2021-08-05  7:09   ` Jan Beulich
2021-08-09 13:31     ` Anthony PERARD
2021-07-01 14:10 ` [XEN PATCH v6 20/31] build: generate "include/xen/compile.h" with filechk Anthony PERARD
2021-08-05  7:20   ` Jan Beulich
2021-08-09 14:27     ` Anthony PERARD
2021-07-01 14:10 ` [XEN PATCH v6 21/31] build: set XEN_BUILD_EFI earlier Anthony PERARD
2021-08-05  7:27   ` Jan Beulich
2021-08-09 15:59     ` Anthony PERARD
2021-08-10  7:44       ` Jan Beulich
2021-07-01 14:10 ` [XEN PATCH v6 22/31] build: fix $(TARGET).efi creation in arch/arm Anthony PERARD
2021-07-01 14:10 ` [XEN PATCH v6 23/31] build: fix arch/x86/node.o rule Anthony PERARD
2021-07-07 16:04   ` Jan Beulich
2021-07-01 14:10 ` [XEN PATCH v6 24/31] build: set ALL_OBJS to main Makefile; move prelink.o to main Makefile Anthony PERARD
2021-07-01 14:10 ` [XEN PATCH v6 25/31] build: remove unneeded deps of x86_emulate.o Anthony PERARD
2021-08-06 16:06   ` Jan Beulich
2021-08-09 16:02     ` Anthony PERARD
2021-07-01 14:10 ` [XEN PATCH v6 26/31] build: clean common temporary files from root makefile Anthony PERARD
2021-07-01 14:10 ` [XEN PATCH v6 27/31] build: prepare to always invoke $(MAKE) from xen/, use $(obj) Anthony PERARD
2021-07-01 14:10 ` [XEN PATCH v6 28/31] build: rework test/livepatch/Makefile Anthony PERARD
2021-07-01 14:10 ` [XEN PATCH v6 29/31] build: build everything from the root dir, use obj=$subdir Anthony PERARD
2021-07-01 17:49   ` Bob Eshleman
2021-07-01 14:10 ` [XEN PATCH v6 30/31] build: introduce if_changed_deps Anthony PERARD
2021-07-01 14:10 ` [XEN PATCH v6 31/31] build,riscv: tell the build system about riscv64/head.S Anthony PERARD
2021-07-01 17:52   ` Bob Eshleman
2021-07-02  4:45   ` Alistair Francis
2021-07-10  0:50   ` Connor Davis

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=3ec5fa48-9079-3e8b-f5d7-41c2b1b923f1@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=julien@xen.org \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

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