Skip to content
This repository was archived by the owner on Nov 4, 2021. It is now read-only.

Conversation

@umarcor
Copy link
Contributor

@umarcor umarcor commented Oct 22, 2020

As commented in #58 (comment), ghdl --version currently shows GHDL 1.0-dev () [Dunoon edition]. That is because the configure/Makefile depend on sources being in a git repo for building the "version description string". However, git_clone removes any reference to git, including .git.

Optionally, envvar GHDL_DESC can be used for providing a fallback description string. That's what I did here. Anyway, feel free to use some better content.

@edbordin
Copy link
Collaborator

Sorry, I missed this for a while! There are other tools that have similar issues when there's no git metadata available. For yosys the git commit is injected with some ad-hoc code, but that mainly got attention because nmigen parses that version string. There are probably various other tools in this build with weird version strings too...

I'm not actually sure what the motivation originally was to strip the .git dir, the scripts already did it when I forked them and I never changed it. I'm not personally motivated to majorly overhaul any of it at the moment though.

If we're going to change this it's probably nice to identify the build a bit more specifically. One option would be to replicate what the GHDL makefile does by running something like this after git_clone:
export GHDL_DESC=$(git -C $UPSTREAM_DIR/$dir_name describe --dirty 2> /dev/null)
Another option would be to just add the datestamp with export GHDL_DESC="open-tool-forge$VERSION". Or I suppose you could have a combination of the two. I'm happy with any combination.

@umarcor
Copy link
Contributor Author

umarcor commented Nov 20, 2020

I combined both of them. I'm not completely sure about what will the result look like, but it will be something we can identify.

@umarcor umarcor force-pushed the ghdl/version branch 2 times, most recently from 112d43d to 0c67b8c Compare November 21, 2020 02:28
@edbordin
Copy link
Collaborator

I tested the linux_x86_64 CI artifact while the CI build was finishing up and the version string looks like this:

$ ./ghdl --version
GHDL 1.0-dev (v0.37.0-1084-g59113550) [Dunoon edition] open-tool-forge.nightly-20201121
 Compiled with GNAT Version: 9.3.0
 mcode code generator
Written by Tristan Gingold.

Copyright (C) 2003 - 2020 Tristan Gingold.
GHDL is free software, covered by the GNU General Public License.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

I'd say that's much better!

I didn't check the other platforms but I'm happy to risk it (seems unlikely they would look different to this and all the basic CI tests just passed). Thanks for your contribution, if you are happy with it I'll merge it!

@umarcor
Copy link
Contributor Author

umarcor commented Nov 21, 2020

I was waiting for CI to pass. I tried doing it without the patch (by setting the newline in GHDL_DESC), but OSX would complain. It was quite a headache; but, in the end, I think that using the patch is much cleaner anyway.

Merge when you want 😉

EDIT

I tested it on Windows. So, the only risk is OSX; it's very low risk 😄 .

@edbordin edbordin merged commit becd6aa into YosysHQ:main Nov 21, 2020
@edbordin
Copy link
Collaborator

Thanks again!

@umarcor umarcor deleted the ghdl/version branch November 21, 2020 05:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants