Dan Magenheimer
2010-Jul-16 16:34 UTC
[Xen-devel] [PATCH] hg ignore libxl lex/yacc detritus
Please apply (this or some cleaner variation) to xen-unstable and xen-4.0-testing so that hg diff is clean after a build. diff -r 57859775f88f .hgignore --- a/.hgignore Fri Jul 09 12:44:53 2010 +0100 +++ b/.hgignore Fri Jul 16 10:24:02 2010 -0600 @@ -317,3 +317,5 @@ ^unmodified_drivers/linux-2.6/.*\.ko$ ^unmodified_drivers/linux-2.6/.*\.mod\.c$ ^LibVNCServer.* +^tools/libxl/libxlu_cfg_l\.h$ +^tools/libxl/libxlu_cfg_y\.h$ +^tools/libxl/libxlu_cfg_l\.c$ +^tools/libxl/libxlu_cfg_y\.c$ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jul-16 16:50 UTC
Re: [Xen-devel] [PATCH] hg ignore libxl lex/yacc detritus
Changeset 21000 checked those files into the tree, so adding them to .hgignore would be very bad form. I''ve missed checking in bits of patches due to that before. If they are getting modified (i.e., regenerated) as part of your build then that is a bug. Possibly the rules to generate them should be commented out in the checked-in Makefile in that case... Whatever, I leave it to Stefano and Ian to make a decision on that. But the right patch won''t touch .hgignore. -- Keir On 16/07/2010 17:34, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:> Please apply (this or some cleaner variation) to > xen-unstable and xen-4.0-testing so that hg diff > is clean after a build. > > diff -r 57859775f88f .hgignore > --- a/.hgignore Fri Jul 09 12:44:53 2010 +0100 > +++ b/.hgignore Fri Jul 16 10:24:02 2010 -0600 > @@ -317,3 +317,5 @@ > ^unmodified_drivers/linux-2.6/.*\.ko$ > ^unmodified_drivers/linux-2.6/.*\.mod\.c$ > ^LibVNCServer.* > +^tools/libxl/libxlu_cfg_l\.h$ > +^tools/libxl/libxlu_cfg_y\.h$ > +^tools/libxl/libxlu_cfg_l\.c$ > +^tools/libxl/libxlu_cfg_y\.c$_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Jul-21 15:26 UTC
Re: [Xen-devel] [PATCH] hg ignore libxl lex/yacc detritus
Keir Fraser writes ("Re: [Xen-devel] [PATCH] hg ignore libxl lex/yacc detritus"):> Changeset 21000 checked those files into the tree, so adding them to > .hgignore would be very bad form. I''ve missed checking in bits of patches > due to that before.Quite.> If they are getting modified (i.e., regenerated) as part of your build then > that is a bug. Possibly the rules to generate them should be commented out > in the checked-in Makefile in that case... Whatever, I leave it to Stefano > and Ian to make a decision on that. But the right patch won''t touch > .hgignore.They should be regenerated if the corresponding input files (libxlu_cfg.l and libxlu_cfg.y) have changed. I think make will avoid rebuilding them if they haven''t. So the make lines shouldn''t be commented out. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Magenheimer
2010-Jul-21 16:42 UTC
RE: [Xen-devel] [PATCH] hg ignore libxl lex/yacc detritus
> Keir Fraser writes ("Re: [Xen-devel] [PATCH] hg ignore libxl lex/yacc > detritus"): > > Changeset 21000 checked those files into the tree, so adding them to > > .hgignore would be very bad form. I''ve missed checking in bits of > patches > > due to that before. > > Quite. > > > If they are getting modified (i.e., regenerated) as part of your > build then > > that is a bug. Possibly the rules to generate them should be > commented out > > in the checked-in Makefile in that case... Whatever, I leave it to > Stefano > > and Ian to make a decision on that. But the right patch won''t touch > > .hgignore. > > They should be regenerated if the corresponding input files > (libxlu_cfg.l and libxlu_cfg.y) have changed. I think make will avoid > rebuilding them if they haven''t. So the make lines shouldn''t be > commented out.Hmmm... that means they get regenerated when an "hg update" is done that updates the input files, resulting in the files being found by hg diff, which appears to be what prompted my .hgignore patch. Why were the output files checked into the tree? The comment "for benefit of prehistoric people" isn''t very illuminating and implies to me that this is just a temporary workaround to avoid adding another tool/download. (And for those without flex(?), will result in an inconsistent build anyway?) At a minimum, good form would require that any patch submitter for the input files also submit the patch for the output files, true? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jul-21 16:59 UTC
Re: [Xen-devel] [PATCH] hg ignore libxl lex/yacc detritus
On 21/07/2010 17:42, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:>> They should be regenerated if the corresponding input files >> (libxlu_cfg.l and libxlu_cfg.y) have changed. I think make will avoid >> rebuilding them if they haven''t. So the make lines shouldn''t be >> commented out. > > Hmmm... that means they get regenerated when an "hg update" > is done that updates the input files, resulting in the > files being found by hg diff, which appears to be > what prompted my .hgignore patch.Yes, tbh I''m not sure we can really rely on the mtimes of the input files and the checked-in generated output files being correctly ordered in a fresh checkout of our repository. I had to comment out the rules to generate our virtual ACPI tables in tools/firmware/hvmloader/acpi when I used to have the output of the ACPI compiler checked in (I''ve got rid of that now). So, really, it may be we either need to remove the checked-in generated files, or comment out the rules that build those generated files. Because we can''t rely on the file modification times being ''right'' in a fresh checkout. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jul-21 17:02 UTC
Re: [Xen-devel] [PATCH] hg ignore libxl lex/yacc detritus
On 21/07/2010 17:59, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:> So, really, it may be we either need to remove the checked-in generated > files, or comment out the rules that build those generated files. Because we > can''t rely on the file modification times being ''right'' in a fresh checkout.My preference would be to delete the checked-in generated files. Then either declare old flex/bison to be simply unsupported, or avoid depending on whatever feature it is that is not supported in those old flex/bison versions. Checking in the generated output is a cheap hack workaround. -- keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Jul-23 15:53 UTC
Re: [Xen-devel] [PATCH] hg ignore libxl lex/yacc detritus
Keir Fraser writes ("Re: [Xen-devel] [PATCH] hg ignore libxl lex/yacc detritus"):> My preference would be to delete the checked-in generated files. Then either > declare old flex/bison to be simply unsupported, or avoid depending on > whatever feature it is that is not supported in those old flex/bison > versions. Checking in the generated output is a cheap hack workaround.I think what we have now is a good compromise. If you have working flex and bison then everything works absolutely fine; if we update the input files we have to remember to run make before committing but that''s not hard (and I do a build test anyway). However providing the checked-in files means that people who have crazy prehistoric versions of flex (1993 in the case of RHEL5 AIUI) or whatever can still build the system. If they work in xen-unstable they may need to knock make on the head, but at least they have a workaround. If we didn''t provide the generated files checked-in, the workaround would be much worse. As an aside, I think there is generally nothing wrong in principle with checking in generated files. It''s something that should be done with proper thought and care, but it''s often appropriate. Eg, automake-using systems should contain configure and configure.in and Makefile.in, so that people who don''t have the right version of automake can still build from a checkout. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel