mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.48k stars 1.59k forks source link

vcs_tag() error output should be silenced if fallback is specified #2240

Open mqudsi opened 7 years ago

mqudsi commented 7 years ago

meson's vcs_tag function is very useful but the fact that it always generates error output if no annotated tag matching the current git revision is found is... unnecessary.

We use actual git revisions as our version numbers for intermediate releases, so we provide the output of git rev-parse HEAD as the value of the fallback parameter to vcs_tag. That works great, but each time meson complains about no annotated tags:

ninja -C debug
ninja: Entering directory `debug'
[1/4] Generating GitVersion.cpp with a custom command.
fatal: No annotated tags can describe 'e5c806c6a0fc0380637b955b018485ccd1214ddd'.
However, there were unannotated tags: try --tags.

If a fallback is provided, I think that output should be silenced.

mqudsi commented 7 years ago

(the fact that it says "fatal" when a fallback is provided is definitely a bug in all cases.)

axxel commented 5 years ago

Could you provide some sample meson.build code that shows what exactly you did and explain what you intended to achieve? I don't understand how/why you would 'provide the output of git...' as the fallback. The fallback is supposed to provide a string in case git could not be executed.

mqudsi commented 5 years ago

@axxel we are thinking of two different failure modes. The failure mode I was referring to is that there are no (matching?) annotated tags but git is executable.

axxel commented 5 years ago

Sorry but I still don't understand what you are trying to do. Can you provide the vcs_tag-line from your meson.build file?

mqudsi commented 5 years ago

I'm trying to use git (tag or revision) as the build tag.

Here's the excerpt from meson.build:

tag_target = vcs_tag(command: ['git', 'rev-parse', 'HEAD'], input: 'utils/GitVersion.cpp', output: 'GitVersion.cpp')

and here's utils/GitVersion.cpp:

#ifndef STRINGIFY
#define STRINGIFY2(x) #x
#define STRINGIFY(x) STRINGIFY2(x)
#endif

#include <string>

namespace neosmart
{
    //std::string GitVersion = STRINGIFY(TPVERSION);
    std::string GitVersion = "@VCS_TAG@";
}
mqudsi commented 5 years ago

Previously I was using the fallback tag:

rev = run_command('git', 'rev-parse', 'HEAD').stdout().strip()
tag_target = vcs_tag(input: 'utils/GitVersion.cpp', output: 'GitVersion.cpp', fallback: rev)

but at some point I switched to the command parameter.

axxel commented 5 years ago

I'm getting close but still don't understand it fully. Please note two things:

  1. your previous approach and your new approach are not equivalent, i.e. they produce different GitVersion.cpp files. Your new approach is the correct way. And it should actually work as is.
  2. the error message you see on the console seems to be from git, not from meson. Unfortunately I don't know enough about git to interpret the error message. If I call git rev-parse HEAD in a tree, I simply get a proper hash code. Googling for the fatal: ... line hints at some missing history in your clone that you'd need to fetch if you wanted info about tags. But I fail to see how rev-parse would care about tags at all.

Do you still get this error text with your new approach? What happens if you simply call git rev-parse HEAD in your source tree?

mqudsi commented 5 years ago

meson internally calls git describe to generate the vcs_tag() output. git describe will throw an error if there are no tags (previously: if there were no annotated tags) that can describe the current revision.

When I filed the bug, I thought "No annotated tags can describe ..." came from meson, not from git. In all cases, it did not come from the code explicitly executed in meson.vim.

The bug was that a fatal error message is generated when a fallback value is provided. An error should not be printed since a fallback means "use this in case of an error".

axxel commented 5 years ago

Thanks for the explanation. So if I got this correct, we have the following situation:

  1. Neither your old nor your new code really 'failed' to perform the replacement of the VCS_TAG string.
  2. Your old code did likely not what you intended but that was because you used the vcs_tag not as it was supposed to be use.
  3. Your new code will in fact not display this 'fatal:...' error because it does not execute git describe.
  4. It would have helped (you and me) if the output on the console was clearly attributed to git.
  5. It would have made the whole debugging of the issue way worse, if meson simply swallowed the error message from git as you suggested.
  6. Please also note that a fallback is always provided, its default value is meson.project_version().

I would propose the following changes to improve the situation:

  1. add some hint to the meson output where the error text came from in case there is one.
  2. rethink the vcs_tag default command used for git repos. A quick check revealed an easy solution: add the --always flag.

@jpakkane would you merge a PR with suggestion two (or simply add that --always flag yourself)?

mqudsi commented 5 years ago

Yup, that sounds about right. Sorry for how hard it was to describe, it's been awhile since I reported the issue. I also believe I changed my usage of vcs_tag() in between, but since forgot which was what.

(Note that I wasn't aware that meson used git describe internally until now, thanks for finding the --always flag, that does make things way easier.)

ernestask commented 5 years ago

Not everyone builds from git, by the way. In those cases, the fatal errors are a mere annoyance.

pwnorbitals commented 3 years ago

Do we have any progress on this issue ? @axxel's suggestions are interesting