verilator icon indicating copy to clipboard operation
verilator copied to clipboard

Verilator builds outside of git self-identify as UNKNOWN.REV, seems a bit strange

Open ldoolitt opened this issue 2 years ago • 9 comments

When building outside of git, in particular from a tarball provided by github for a tag, the result self-identifies as e.g., Verilator 4.218 2022-01-17 rev UNKNOWN.REV. It's clear how this happens: git describe fails. But the whole point of using a tagged version is to know precisely and concisely what version it is. Flagging it as "unknown" seems out-of-place, at best.

I see this "all the time". There are seven github issues where the version is reported like this (and nobody complained). Stock Debian Buster (yes, obsolete) Verilator is "Verilator 4.010 2019-01-27 rev UNKNOWN_REV".

I'd be happy to revise the python program src/config_rev, if there's any rough consensus on how to improve this. IMHO, a simple "Verilator 4.010 2019-01-27" would be enough. "Omit needless words" - Strunk and White. If we think people will write (or have written) fragile parsers for this string, maybe repeat the tag: "Verilator 4.010 2019-01-27 rev 4.010".

ldoolitt avatar Mar 14 '22 19:03 ldoolitt

I'm fine with us changing to e.g. "Verilator 4.218 2022-01-17" when git describe fails, and would welcome a related patch. I would hope no one is triggering off more than the version number.

Note that many distros put patches on top of the base version, so there's no way to know what exactly is in there if git describe fails. I know some other packages (e.g. emacs) include build numbers for those cases. I'm not that worried about this, but perhaps merits discussion.

wsnyder avatar Mar 14 '22 19:03 wsnyder

Note that many distros put patches on top of the base version, so there's no way to know what exactly is in there if git describe fails.

True. There's a lot the version string doesn't cover, including the c++ compiler and the options given to configure. For distros, maybe we can give a mechanism and best-practices for them to ID their result as modified (or not).

ldoolitt avatar Mar 14 '22 20:03 ldoolitt

The .gitattributes file supports a tag export-subst, which will do a string-replace on the specified files, when they are exported. This can be used to insert the describe string into the source files, when they are exported from git.

I used this in a repo I administer, where we use a shell script to generate the version info, and the script (here) is able to test whether it's being run from exported code (and could even add that knowledge to the generated version string). Here's what that repo does:

# If the source was exported (via 'git archive')
if [[ '$Format:%%$' == "%" ]]; then
    GITCOMMIT='$Format:%h$'
    if [[ '$Format:%D$' =~ tag:\ ([^ ]+) ]]; then
	GITDESCR="${BASH_REMATCH[1]}"
    else
	GITDESCR="archive-g${GITCOMMIT}"
    fi
else

quark17 avatar Mar 14 '22 22:03 quark17

@ldoolitt did you want to submit a patch to remove the UNKNOWN or otherwise?

wsnyder avatar Mar 22 '22 01:03 wsnyder

I like the idea presented by @quark17, but haven't found time to pursue that and submit a patch. I wasn't familiar with that feature available on github, and will want to run some experiments.

ldoolitt avatar Mar 22 '22 04:03 ldoolitt

I made a test/sandbox repo, at https://github.com/ldoolitt/vvtest1. Some of the desired features are there, and y'all are welcome to take a look and offer critiques. I don't think I'm quite ready to fork and patch Verilator.

ldoolitt avatar Apr 03 '22 04:04 ldoolitt

Note running a script on checkout (which I think vvtest1 needs) is disabled in some sites due to security policies.

wsnyder avatar May 12 '22 01:05 wsnyder

The requisite features that make vvtest1 "work", also used by @quark17's bsc repo, are really core git features. It took me a while to figure that out. Github's source code assets (.tar.gz) for a release are indistinguishable from the output of git archive. That means the version-labeling features of a project are (at least partially) testable on a local machine's git repo, without involving github.

Documentation for this part of git is rather diffuse; I spent some time bouncing around between man git-archive man gitattributes man git-log See in particular the export-subst attribute and the placeholders listed in the "PRETTY FORMATS" section of man git-log.

Of course there's no clear expectation that any particular "site" will use an un-modified git (archive) program. But I don't think security policies for scripting will enter into it. Github itself is tested and works in this regard.

ldoolitt avatar Sep 11 '22 21:09 ldoolitt

I also just now tested this git feature set on a locally-hosted gitlab instance. Works fine.

ldoolitt avatar Sep 12 '22 00:09 ldoolitt

To work around this issue, Debian packaging of 5.006 uses the following one-liner in debian/rules:

        sed -i 's/UNKNOWN_REV/(Debian $(DEB_VERSION))/g' src/config_rev

The resulting binary then self-identifies as

Verilator 5.006 2023-01-22 rev (Debian 5.006-1)

That's certainly simpler and more direct than the fancy git-archive features I experimented with. Other packagers might be able to find this bug (even if it 's closed?), and take lessons. If a Real Verilator Maintainer thinks this approach is somehow wrong-headed, please let Carsten or me know.

ldoolitt avatar Jan 29 '23 22:01 ldoolitt

Makes sense to me.

wsnyder avatar Jan 29 '23 22:01 wsnyder

Also connected to pull request #4954

ldoolitt avatar Mar 11 '24 17:03 ldoolitt