Here's a patch for discussion. Since I am parsing Dillo logs for debugging, I thought it would be neat if Dillo logged the mercurial revision. Patch attached. Thoughts? Does anyone know a better way to get the revision for a particular tag? Piping "hg tags" through a sed script is rather kludgy but I can't find a way to get mercurial to tell me directly. I log the qparent revision as well as the tip revision because I manage patches with Mercurial queues, so my qparent revision is the vanilla tip revision that I am currently based on. Also, I used printf() to log the revisions because neither MSG nor MSG_WARN produced anything. I don't know why. Can anyone explain? Regards, Jeremy Henty
On Mon, Feb 16, 2009 at 11:09:21PM +0000, Jeremy Henty wrote:
Here's a patch for discussion. Since I am parsing Dillo logs for debugging, I thought it would be neat if Dillo logged the mercurial revision. Patch attached.
Thoughts? Does anyone know a better way to get the revision for a particular tag? Piping "hg tags" through a sed script is rather kludgy but I can't find a way to get mercurial to tell me directly.
I like the idea, but wouldn't something like "hg id" be more appropriate? It shows the current revision which is not necessarily tip. Cheers, Johannes
On Tue, Feb 17, 2009 at 06:40:01PM +0100, Joerg Sonnenberger wrote:
Please ensure that this is not part of releases and/or if the .hg directory is missing.
It already does this. If you are not in a Mercurial repository (or if Mercurial is not installed) then the "if hg id ..." test fails and HG is #defined to 0 instead of 1. The extra logging code is wrapped in a "#if HG" so it will be removed by the preprocessor. I just tested this by temporarily renaming .hg and rebuilding and it worked as intended. Regards, Jeremy Henty
On Tue, Feb 17, 2009 at 05:54:17PM +0000, Jeremy Henty wrote:
On Tue, Feb 17, 2009 at 06:40:01PM +0100, Joerg Sonnenberger wrote:
Please ensure that this is not part of releases and/or if the .hg directory is missing.
It already does this. If you are not in a Mercurial repository (or if Mercurial is not installed) then the "if hg id ..." test fails and HG is #defined to 0 instead of 1. The extra logging code is wrapped in a "#if HG" so it will be removed by the preprocessor. I just tested this by temporarily renaming .hg and rebuilding and it worked as intended.
Point is, release build should never differ whether hg is installed or not. I know that hg does the check internally, but that also depends on that no other hg binary can be found. Joerg
On Tue, Feb 17, 2009 at 07:49:33PM +0100, Joerg Sonnenberger wrote:
Point is, release build should never differ whether hg is installed or not.
I absolutely agree, but if by "release build" you mean "a build not in a Mercurial repository" then that is already true of the current version of the patch. If you're not in a Mercurial repository then exactly the same code is compiled whether Mercurial is installed or not.
I know that hg does the check internally, but that also depends on that no other hg binary can be found.
Do you mean that we should worry that there might be an hg in $PATH that is not Mercurial? Is it really necessary to be that cautious? If we must handle host OSes that break our attempts to auto-detect hg then we will have to make the code conditional on a new configure flag (eg. --enable-hg-logging), with the proviso that if you supply the flag then the obligation is on you to ensure that your hg is a properly working Mercurial executable. That's easy to implement. I hope I don't seem obstinate, but I am finding it hard to see the scenario that supposedly breaks the current patch. (Unless the scenario is "hg might not be Mercurial" in which case I have already described what I think is the only sensible fix.) Regards, Jeremy Henty
On Tue, Feb 17, 2009 at 07:52:13PM +0000, Jeremy Henty wrote:
I know that hg does the check internally, but that also depends on that no other hg binary can be found.
Do you mean that we should worry that there might be an hg in $PATH that is not Mercurial? Is it really necessary to be that cautious?
It would be nice, yes. What about just wrapping this into if [ -d .hg ]; ... fi or something like that? Joerg
On Tue, Feb 17, 2009 at 09:08:33PM +0100, Joerg Sonnenberger wrote:
On Tue, Feb 17, 2009 at 07:52:13PM +0000, Jeremy Henty wrote:
Do you mean that we should worry that there might be an hg in $PATH that is not Mercurial? Is it really necessary to be that cautious?
It would be nice, yes. What about just wrapping this into if [ -d .hg ]; ... fi or something like that?
I'm completely convinced that this really adds any safety, but it's easy enough to do. Revised patch attached. BTW: the autoconf documentation frowns on this sort of thing, from (autoconf.info)Limitations of Builtins: `test' (files) To enable `configure' scripts to support cross-compilation, they shouldn't do anything that tests features of the build system instead of the host system. So I guess if we want to be truly GNU-ly correct we should disable this code unless the builder specifies some enable-* configure flag. However, the code won't break any cross-compilation so it's not critical. Regards, Jeremy Henty
On Wed, Feb 18, 2009 at 10:49:36AM +0000, Jeremy Henty wrote:
I'm completely convinced that this really adds any safety, but it's easy enough to do. Revised patch attached.
Looks good.
BTW: the autoconf documentation frowns on this sort of thing, from (autoconf.info)Limitations of Builtins:
`test' (files) To enable `configure' scripts to support cross-compilation, they shouldn't do anything that tests features of the build system instead of the host system.
This only applies for checking things in the host filesystem. E.g. testing for the existance of /etc/passwd would be bad for cross-compilation.
--- a/src/dillo.cc +++ b/src/dillo.cc @@ -211,6 +211,11 @@ int width = D_GEOMETRY_DEFAULT_WIDTH, height = D_GEOMETRY_DEFAULT_HEIGHT; char **opt_argv;
+#if HG + printf("HG current: %s\n", HG_CURRENT); + printf("HG qparent: %s\n", HG_QPARENT); +#endif +
Any reason why this are two lines? Joerg
On Wed, Feb 18, 2009 at 02:44:15PM +0100, Joerg Sonnenberger wrote:
On Wed, Feb 18, 2009 at 10:49:36AM +0000, Jeremy Henty wrote:
BTW: the autoconf documentation frowns on this sort of thing,
This only applies for checking things in the host filesystem.
Yes, I realised after posting that since the code I am adding is intended to reflect the build system, no matter what the host system is, cross-compilation issues just do not arise.
+#if HG + printf("HG current: %s\n", HG_CURRENT); + printf("HG qparent: %s\n", HG_QPARENT); +#endif +
Any reason why this are two lines?
No specific reason, but on general principle I usually write one line per record of information to make it easier to process the results with grep/sed etc. Regards, Jeremy Henty
participants (3)
-
joerg.sonnenberger＠web.de -
Johannes.Hofmann＠gmx.de -
onepoint＠starurchin.org