| Message ID | 1368541851-31089-1-git-send-email-jezz@sysmic.org |
|---|---|
| State | Superseded |
| Headers | show |
Dear Jérôme Pouiller, On Tue, 14 May 2013 16:30:50 +0200, Jérôme Pouiller wrote: > -$(BUILD_DIR)/.root: > +$(STAMP_DIR)/skeleton-target-installed: > mkdir -p $(TARGET_DIR) > rsync -a \ > --exclude .empty --exclude .svn --exclude .git \ > --exclude .hg --exclude=CVS --exclude '*~' \ > $(TARGET_SKELETON)/ $(TARGET_DIR)/ > cp support/misc/target-dir-warning.txt $(TARGET_DIR_WARNING_FILE) > + mkdir -p $(STAMP_DIR) > touch $@ I'm generally ok with the idea, but I don't like this mkdir. There is already a mkdir that creates the stamp file directory, in the main Makefile. Thomas
On Tuesday 14 May 2013 17:07:21 Thomas Petazzoni wrote: > Dear Jérôme Pouiller, > > On Tue, 14 May 2013 16:30:50 +0200, Jérôme Pouiller wrote: > > -$(BUILD_DIR)/.root: > > > > +$(STAMP_DIR)/skeleton-target-installed: > > mkdir -p $(TARGET_DIR) > > rsync -a \ > > > > --exclude .empty --exclude .svn --exclude .git \ > > --exclude .hg --exclude=CVS --exclude '*~' \ > > $(TARGET_SKELETON)/ $(TARGET_DIR)/ > > > > cp support/misc/target-dir-warning.txt $(TARGET_DIR_WARNING_FILE) > > > > + mkdir -p $(STAMP_DIR) > > > > touch $@ > > I'm generally ok with the idea, but I don't like this mkdir. There is > already a mkdir that creates the stamp file directory, in the main > Makefile. I also dislike this mkdir. My problem is to be sure $(STAMP_DIR) is created before $(STAMP_DIR)/skeleton-target-installed. I can change order of dependencies in dirs target but this solution is too fragile. I cannot just add a dependency between $(STAMP_DIR)/skeleton-target-installed and $(STAMP_DIR). I can add a stampfile for $(STAMP_DIR) but it looks a little complicated. I can also use an intermediate phony target (like dirs target) but it also looks complicated. Any better idea?
On 14/05/13 17:07, Thomas Petazzoni wrote: > Dear Jérôme Pouiller, > > On Tue, 14 May 2013 16:30:50 +0200, Jérôme Pouiller wrote: > >> -$(BUILD_DIR)/.root: >> +$(STAMP_DIR)/skeleton-target-installed: >> mkdir -p $(TARGET_DIR) >> rsync -a \ >> --exclude .empty --exclude .svn --exclude .git \ >> --exclude .hg --exclude=CVS --exclude '*~' \ >> $(TARGET_SKELETON)/ $(TARGET_DIR)/ >> cp support/misc/target-dir-warning.txt $(TARGET_DIR_WARNING_FILE) >> + mkdir -p $(STAMP_DIR) >> touch $@ > > I'm generally ok with the idea, but I don't like this mkdir. There is > already a mkdir that creates the stamp file directory, in the main > Makefile. I don't have an issue with a few redundant mkdir -p calls. I would like to see a $(Q) in front of it though - but then it should be in front of the rest as well, and with a $(MESSAGE) call. Regards, Arnout
Arnout, All, On 2013-05-14 23:52 +0200, Arnout Vandecappelle spake thusly: > On 14/05/13 17:07, Thomas Petazzoni wrote: > >Dear Jérôme Pouiller, > > > >On Tue, 14 May 2013 16:30:50 +0200, Jérôme Pouiller wrote: > > > >>-$(BUILD_DIR)/.root: > >>+$(STAMP_DIR)/skeleton-target-installed: > >> mkdir -p $(TARGET_DIR) > >> rsync -a \ > >> --exclude .empty --exclude .svn --exclude .git \ > >> --exclude .hg --exclude=CVS --exclude '*~' \ > >> $(TARGET_SKELETON)/ $(TARGET_DIR)/ > >> cp support/misc/target-dir-warning.txt $(TARGET_DIR_WARNING_FILE) > >>+ mkdir -p $(STAMP_DIR) > >> touch $@ > > > >I'm generally ok with the idea, but I don't like this mkdir. There is > >already a mkdir that creates the stamp file directory, in the main > >Makefile. > > I don't have an issue with a few redundant mkdir -p calls. I concur with Thomas: we should ensure the stamp-dir is already created in a generic way, it's kind of an internal stuff, which should not be visible to users (I mean: packages, and stuff like that, not actual persons). That we need to sprinkle the code with a few 'mkdir' here and ther "just in case" is an indication we do not fully masterise the chain of events, and is really not nice. The fact that the 'mkdir' is needed in the first place is because of this rulle in the top-level Makefile: dirs: [...] $(TARGET_DIR) [...] $(STAMP_DIR) which will have 'make' run the $(TARGET_DIR) goal before the $(STAMP_DIR) goal. This can be solved in two ways: the nice one, and the not-so-nice one: - have an additional rule: $(TARGET_DIR): $(STAMP_DIR) <- the nice way - invert the order of goals in the 'dirs' rule <- the not-so-nice way Regards, Yann E. MORIN.
Jérôme, All, On 2013-05-14 17:27 +0200, Jérôme Pouiller spake thusly: > On Tuesday 14 May 2013 17:07:21 Thomas Petazzoni wrote: > > Dear Jérôme Pouiller, > > > > On Tue, 14 May 2013 16:30:50 +0200, Jérôme Pouiller wrote: > > > -$(BUILD_DIR)/.root: > > > > > > +$(STAMP_DIR)/skeleton-target-installed: > > > mkdir -p $(TARGET_DIR) > > > rsync -a \ > > > > > > --exclude .empty --exclude .svn --exclude .git \ > > > --exclude .hg --exclude=CVS --exclude '*~' \ > > > $(TARGET_SKELETON)/ $(TARGET_DIR)/ > > > > > > cp support/misc/target-dir-warning.txt $(TARGET_DIR_WARNING_FILE) > > > > > > + mkdir -p $(STAMP_DIR) > > > > > > touch $@ > > > > I'm generally ok with the idea, but I don't like this mkdir. There is > > already a mkdir that creates the stamp file directory, in the main > > Makefile. > I also dislike this mkdir. My problem is to be sure $(STAMP_DIR) is created > before $(STAMP_DIR)/skeleton-target-installed. > > I can change order of dependencies in dirs target but this solution is too > fragile. Agreed, this is not-so-nice. > I cannot just add a dependency between $(STAMP_DIR)/skeleton-target-installed > and $(STAMP_DIR). Why not? Like: $(STAMP_DIR)/skeleton-target-installed: $(STAMP_DIR) which to me looks like the sane solution (and not what I previosuly answered Arnout about $(TARGET_DIR)) Regards, Yann E. MORIN.
On Wednesday 15 May 2013 00:23:24 Yann E. MORIN wrote: [...] > > I cannot just add a dependency between > > $(STAMP_DIR)/skeleton-target-installed and $(STAMP_DIR). > > Why not? Like: > $(STAMP_DIR)/skeleton-target-installed: $(STAMP_DIR) > > which to me looks like the sane solution (and not what I previosuly > answered Arnout about $(TARGET_DIR)) Because, $(STAMP_DIR)/skeleton-target-installed will be rebuilt each time a file is added or removed from $(STAMP_DIR).
On Wednesday 15 May 2013 08:19:10 Jérôme Pouiller wrote: > On Wednesday 15 May 2013 00:23:24 Yann E. MORIN wrote: > [...] > > > > I cannot just add a dependency between > > > $(STAMP_DIR)/skeleton-target-installed and $(STAMP_DIR). > > > > Why not? Like: > > $(STAMP_DIR)/skeleton-target-installed: $(STAMP_DIR) > > > > which to me looks like the sane solution (and not what I previosuly > > answered Arnout about $(TARGET_DIR)) > > Because, $(STAMP_DIR)/skeleton-target-installed will be rebuilt each time a > file is added or removed from $(STAMP_DIR). But, we may use an intermediate phony target: $(STAMP_DIR)_phony: $(STAMP_DIR) .PHONY: $(STAMP_DIR)_phony $(STAMP_DIR)/skeleton-target-installed: $(STAMP_DIR)_phony (not tested, but should work)
On 2013-05-15 08:19 +0200, Jérôme Pouiller spake thusly: > On Wednesday 15 May 2013 00:23:24 Yann E. MORIN wrote: > [...] > > > I cannot just add a dependency between > > > $(STAMP_DIR)/skeleton-target-installed and $(STAMP_DIR). > > > > Why not? Like: > > $(STAMP_DIR)/skeleton-target-installed: $(STAMP_DIR) > > > > which to me looks like the sane solution (and not what I previosuly > > answered Arnout about $(TARGET_DIR)) > Because, $(STAMP_DIR)/skeleton-target-installed will be rebuilt each time a > file is added or removed from $(STAMP_DIR). Indeed. Sigh. Too short-sighted I was... Regards, Yann E. MORIN.
Jérôme, All, On 2013-05-15 08:27 +0200, Jérôme Pouiller spake thusly: > On Wednesday 15 May 2013 08:19:10 Jérôme Pouiller wrote: > > On Wednesday 15 May 2013 00:23:24 Yann E. MORIN wrote: > > [...] > > > > > > I cannot just add a dependency between > > > > $(STAMP_DIR)/skeleton-target-installed and $(STAMP_DIR). > > > > > > Why not? Like: > > > $(STAMP_DIR)/skeleton-target-installed: $(STAMP_DIR) > > > > > > which to me looks like the sane solution (and not what I previosuly > > > answered Arnout about $(TARGET_DIR)) > > > > Because, $(STAMP_DIR)/skeleton-target-installed will be rebuilt each time a > > file is added or removed from $(STAMP_DIR). > But, we may use an intermediate phony target: > $(STAMP_DIR)_phony: $(STAMP_DIR) > .PHONY: $(STAMP_DIR)_phony > $(STAMP_DIR)/skeleton-target-installed: $(STAMP_DIR)_phony Or an order-only rule: $(STAMP_DIR)/skeleton-target-installed: | $(STAMP_DIR) (not tested, but it's used in the crosstool-NG backend) Regards, Yann E. MORIN.
diff --git a/Makefile b/Makefile index a57975e..73a7c01 100644 --- a/Makefile +++ b/Makefile @@ -429,16 +429,17 @@ ifeq ($(BR2_ROOTFS_SKELETON_CUSTOM),y) TARGET_SKELETON=$(BR2_ROOTFS_SKELETON_CUSTOM_PATH) endif -$(BUILD_DIR)/.root: +$(STAMP_DIR)/skeleton-target-installed: mkdir -p $(TARGET_DIR) rsync -a \ --exclude .empty --exclude .svn --exclude .git \ --exclude .hg --exclude=CVS --exclude '*~' \ $(TARGET_SKELETON)/ $(TARGET_DIR)/ cp support/misc/target-dir-warning.txt $(TARGET_DIR_WARNING_FILE) + mkdir -p $(STAMP_DIR) touch $@ -$(TARGET_DIR): $(BUILD_DIR)/.root +$(TARGET_DIR): $(STAMP_DIR)/skeleton-target-installed STRIP_FIND_CMD = find $(TARGET_DIR) ifneq (,$(call qstrip,$(BR2_STRIP_EXCLUDE_DIRS)))
To be coherent with stamps used by toolchains, file $(BUILD)/.root is now named $(STAMPS)/target-skeleton-installed. Signed-off-by: Jérôme Pouiller <jezz@sysmic.org> --- Makefile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)