| Message ID | 4CDD502D.9060405@gnu.org |
|---|---|
| State | New |
| Headers | show |
Quoting Paolo Bonzini <bonzini@gnu.org>: > On 11/12/2010 03:13 PM, Joern Rennecke wrote: ... >> Are you still planning to do the review in the near future? > > Yes. Your patch has the disadvantage that a normal "make" will not > do the check anymore, because you removed the dependency from all to > s-tm-texi (which previously went all -> info -> gccint.info -> > tm.texi -> s-tm-texi). We still have a gccint.info -> new-tm.texi -> s-tm-texi link. > What about this instead, which breaks the circular dependency by > making tm.texi phony. This way "$(MAKE) tm.texi" is effectively > a synonym of "$(MAKE s-tm-texi)". > > --- Makefile.in (revision 166517) > +++ Makefile.in (working copy) > @@ -3688,7 +3688,10 @@ s-constrs-h: $(MD_DEPS) build/genpreds$( > $(STAMP) s-constrs-h > > target-hooks-def.h: s-target-hooks-def-h; @true > -tm.texi: s-tm-texi; @true > + > +all: tm.texi > +tm.texi: force > + $(MAKE) s-tm-texi This looks unsafe in a parallel make context. make could decide to build tm.texi and s-tm-texi simultaneously. Then in order to build tm.texi, it will invoke another make that builds s-tm-texi, too.
On 11/12/2010 04:02 PM, Joern Rennecke wrote: > Quoting Paolo Bonzini <bonzini@gnu.org>: > >> On 11/12/2010 03:13 PM, Joern Rennecke wrote: > ... >>> Are you still planning to do the review in the near future? >> >> Yes. Your patch has the disadvantage that a normal "make" will not >> do the check anymore, because you removed the dependency from all to >> s-tm-texi (which previously went all -> info -> gccint.info -> >> tm.texi -> s-tm-texi). > > We still have a gccint.info -> new-tm.texi -> s-tm-texi link. Ok, I missed the last hunk. That is unfortunately wrong because you would force people who compile a release tarball to run makeinfo (because new-tm.texi) is not shipped. I think your patch can be okay if you leave TEXI_GCCINT_FILES aside, and put an additional dependency @MAINT@ all: new-tm.texi Paolo
Quoting Paolo Bonzini <bonzini@gnu.org>: > I think your patch can be okay if you leave TEXI_GCCINT_FILES aside, > and put an additional dependency > > @MAINT@ all: new-tm.texi That has some other drawbacks - you could end up using inconsistent / out-of date documentation if you haven't explicitly specified maintainer mode. I think the solution is to have the .info files depend in the stamp file, not on tm.texi or new-tm.texi. In fact, I think I already did this before, in version that reduced unnecessary rebuilds by paying more attention to stamp file usage - I just thought it'll be easier to review these Makefile changes in smaller increments. But if a partial fix involves another regression, it's probably better to fix the entire thing. I'll go digging for what I had there.
P.S.: Of course it's the other way round: doc/tm.texi depends on the stampfile, the info files depend on doc/tm.texi . If the documentation is up-to-date, the stampfile gets made without complaint, and doc/tm.texi won't get touched, so if your info files are newer than that, they won't need top be rebuilt.
On 11/12/2010 04:51 PM, Joern Rennecke wrote: > But if a partial fix involves another regression, it's probably better to > fix the entire thing. I'll go digging for what I had there. Sure. Paolo
Quoting Joern Rennecke <amylaar@spamcop.net>: > P.S.: Of course it's the other way round: > doc/tm.texi depends on the stampfile, Oops, there is the circular dependency again. What we actually need is a timestamp that says that the consistency has been checked, and which depends on doc/tm.texi - that's our current s-tm-texi - plus a generated file that depends on s-tm-texi, but bears the file system time stamp of doc/tm.texi . I.e. the s-tm-texi rule should make this file. Can we use cp -p for this portably? In principle we could also use some combination of time and ls and shell substitutions, that would use less disk space, but I fear the portability issues there could be worse. Or should I better try to use double-colon rules? I.e. TEXI_GCCINT_FILES would include [doc/]tm.texi, but all the files that depend on TEXI_GCCINT_FILES would do so with a double-colon rule, and in an earlier double-colon rule, they'd all depend on s-tm-texi, with empty commands, to make sure doc/tm.texi is up-to-date. Using double-colon rules would avoid portability concerns how to copy a timestamp, but would force as to write all the commands to generate the files that depend on tm.texi in the rule that state the dependency.
* Joern Rennecke wrote on Sun, Nov 14, 2010 at 03:46:29PM CET: > What we actually need is a timestamp that says that the consistency has been > checked, and which depends on doc/tm.texi - that's our current s-tm-texi - > plus a generated file that depends on s-tm-texi, but bears the file system > time stamp of doc/tm.texi . > I.e. the s-tm-texi rule should make this file. > Can we use cp -p for this portably? Not likely; see the discussion in 'info Autoconf --index cp'. > In principle we could also use some combination of time and ls and shell > substitutions, that would use less disk space, but I fear the portability > issues there could be worse. Yes, that sounds a lot worse. > Or should I better try to use double-colon rules? If that helps, I suppose yes; we assume GNU make in this makefile anyway. Hope that helps. Cheers, Ralf
Quoting Ralf Wildenhues <Ralf.Wildenhues@gmx.de>: > * Joern Rennecke wrote on Sun, Nov 14, 2010 at 03:46:29PM CET: ... >> Or should I better try to use double-colon rules? > > If that helps, I suppose yes; we assume GNU make in this makefile > anyway. I just realized there is a third alternative: we could exploit the fact that the same file is handled by make as different targets if named with different pathnames, i.e. [$(srcdir)/doc/]tm.texi and $(srcdir)/doc/../doc/tm.texi are different make targets. I'm not sure if that would be exploiting a stable feature or a bug/limitation that might disappear from GNU make before the FSF GPL/GFDL problems are resolved. If we could use a file as differetn make targets in this fashion, that would be a lot less pain to implement than using double-colon rules.
* Joern Rennecke wrote on Sun, Nov 14, 2010 at 04:35:02PM CET: > I just realized there is a third alternative: we could exploit the fact > that the same file is handled by make as different targets if named > with different pathnames, i.e. > [$(srcdir)/doc/]tm.texi and $(srcdir)/doc/../doc/tm.texi are > different make targets. > I'm not sure if that would be exploiting a stable feature or a bug/limitation > that might disappear from GNU make before the FSF GPL/GFDL problems > are resolved. It's not likely that this will disappear, but code like that will jump out at the casual reader months from now as likely-buggy. So that would definitely deserve a comment. Maybe also a make-time (or configure-time) test that this feature/bug has not changed? Note also that in case you're playing a similar game with ./file vs. $(srcdir)/file that will hurt in-tree builds of GCC (which, while advised against, would still be nice to have working). Cheers, Ralf
Quoting Ralf Wildenhues <Ralf.Wildenhues@gmx.de>: > * Joern Rennecke wrote on Sun, Nov 14, 2010 at 03:46:29PM CET: >> What we actually need is a timestamp that says that the consistency has been >> checked, and which depends on doc/tm.texi - that's our current s-tm-texi - >> plus a generated file that depends on s-tm-texi, but bears the file system >> time stamp of doc/tm.texi . >> I.e. the s-tm-texi rule should make this file. >> Can we use cp -p for this portably? > > Not likely; see the discussion in 'info Autoconf --index cp'. This talks about the possibility that the timestamp might end up appearing up to one second too old. So in theory, you could end up with the info files not being rebuilt even though the .texi file is newer. In practice, the info files would need to be rebuilt if someone changes one of the input files of $(builddir)/{new-,}tm.texi, rebuilds {new-,}tm.texi, and copies it manually to $(srcdir)/doc/tm.texi. A turn around time of less than a second for that process seems rather unlikely, so if the only worry about cp -p is truncating the time to look up to a second older, I think it is portable enough for this purpose.
On 11/14/2010 09:46 AM, Joern Rennecke wrote: > Quoting Joern Rennecke <amylaar@spamcop.net>: > >> P.S.: Of course it's the other way round: >> doc/tm.texi depends on the stampfile, > Oops, there is the circular dependency again. > > What we actually need is a timestamp that says that the consistency has > been > checked, and which depends on doc/tm.texi - that's our current s-tm-texi - > plus a generated file that depends on s-tm-texi, but bears the file system > time stamp of doc/tm.texi . > I.e. the s-tm-texi rule should make this file. > Can we use cp -p for this portably? > In principle we could also use some combination of time and ls and shell > substitutions, that would use less disk space, but I fear the portability > issues there could be worse. Just in case anyone's looking for a good project, I figured out a VERY long time ago that the true long-term solution to practically all problems I've ever encountered in Makefile design, including this one, lies in creating an extension to "make". Unfortunately I've never gotten around to digging into the internals of anyone's version of 'make' in order to figure out where to implement this, and the GNU make people were completely uninterested, since they didn't get why it was a good idea. 'make' badly needs a mechanism whereby the Makefile programmer can specify a code fragment for a given target A which answers the question 'is A up to date?'. The default implementation would be to check the timestamp relative to the timestamps of the dependencies, but the Makefile programmer should *be able to specify arbitrary rules*. One would want them to be fast, but that's the Makefile programmer's problem. :-) They should be able to depend on the boolean answers to the question of whether the prereqs are up to date. It would be easier to do this in make than by the generation of many artificial, breakage-prone timestamping and phony targets; it would also render the majority of GNU make extensions irrelevant. In the meantime, Joern's patch of 2010-11-14 looks good to me.
On 11/15/2010 03:52 AM, Nathanael Nerode (GCC) wrote:
> In the meantime, Joern's patch of 2010-11-14 looks good to me.
Joern, take this as an approval.
Nathanael, welcome back!
Paolo
Hello Nathanael, * Nathanael Nerode (GCC) wrote on Mon, Nov 15, 2010 at 03:52:44AM CET: > Just in case anyone's looking for a good project, I figured out a VERY > long time ago that the true long-term solution to practically all > problems I've ever encountered in Makefile design, including this one, > lies in creating an extension to "make". Unfortunately I've never > gotten around to digging into the internals of anyone's version of > 'make' in order to figure out where to implement this, and the GNU make > people were completely uninterested, since they didn't get why it was a > good idea. > 'make' badly needs a mechanism whereby the Makefile programmer can > specify a code fragment for a given target A which answers the question > 'is A up to date?'. I assume you are hinting at <http://thread.gmane.org/gmane.comp.gnu.make.bugs/1007>. The reply from Paul is still mostly correct though: make walks the complete dependency tree down and then back up. When walking back up, the mechanism you desire is easily implemented in a small shell command (as shown in the example of Paul's reply). When first walking down, you just need to ensure that the graph really is a tree. There are (at least) two choices you can make at that time, and in GNU make you can implement them with arbitrary shell statements: - should T be updated if P is newer? T : $(shell if $$condition; then echo P; fi) - should T, if it is to be updated, be updated after P is updated (order-only dependency, since GNU make 3.80)? T : | $(shell if $$condition; then echo P; fi) So, in summary, I think what you want is already possible, albeit maybe not in a concise notation. Cheers, Ralf
On 11/20/2010 04:00 PM, Ralf Wildenhues wrote: > The reply from Paul is still mostly correct though: make walks the > complete dependency tree down and then back up. When walking back up, > the mechanism you desire is easily implemented in a small shell command > (as shown in the example of Paul's reply). It's dependent on timestamp files which can be poked, prodded, and otherwise convinced to do the wrong thing by the user. It's fundamentally *not robust*, is the main issue. > When first walking down, you just need to ensure that the graph really > is a tree. There are (at least) two choices you can make at that time, > and in GNU make you can implement them with arbitrary shell statements: > > - should T be updated if P is newer? > > T : $(shell if $$condition; then echo P; fi) The problematic part is that you're scattering bogus timestamp files (P!) all over the tree. This makes it very brittle. I suppose one could construct a special subtree of timestamps which is blown away fresh with each invocation.... that would make it *slightly more* robust. Not *much* more. > - should T, if it is to be updated, be updated after P is updated > (order-only dependency, since GNU make 3.80)? > > T : | $(shell if $$condition; then echo P; fi) > > So, in summary, I think what you want is already possible, albeit maybe > not in a concise notation. It's not so much the notation (although that is a problem) as the scattering of bogus timestamp files. It is true that by introducing an auxilliary file to represent *each* logical state, and manipulating its timestamp by hand to be "true" or "false", you can "solve" the problem, but this is *not robust*, because the timestamp files are exposed to general programmer manipulation. A special timestamp subdirectory tree would ameliorate that somewhat. It is also, incidentally, slow, due to the heavy reliance on filesystem access and modification, for things for which the filesystem is, fundamentally, quite irrelevant. I suppose it could be sped up with a mounted ramdisk for the timestamp tree, but you begin to see how ridiculous this set of kludges is.
--- Makefile.in (revision 166517) +++ Makefile.in (working copy) @@ -3688,7 +3688,10 @@ s-constrs-h: $(MD_DEPS) build/genpreds$( $(STAMP) s-constrs-h target-hooks-def.h: s-target-hooks-def-h; @true -tm.texi: s-tm-texi; @true + +all: tm.texi +tm.texi: force + $(MAKE) s-tm-texi s-target-hooks-def-h: build/genhooks$(build_exeext) $(RUN_GEN) build/genhooks$(build_exeext) > tmp-target-hooks-def.h