Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a VERSION file and fix compilation #15

Merged
merged 1 commit into from
Jul 3, 2023
Merged

Add a VERSION file and fix compilation #15

merged 1 commit into from
Jul 3, 2023

Conversation

mmichaliINTC
Copy link
Contributor

In the archive with code automatically generated by GitHub the repository files are missing, so the VERSION cannot be created basing on the commits history and tags.

Add a static VERSION file to enable compilation of code without the access to git repository.

VERSION Outdated
@@ -0,0 +1 @@
0.9.0+
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the "+" suffix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something in between two releases, definitely not 0.9.0 - this would be changed to 0.9.1 on the upcoming release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, got it

Makefile Outdated
VERSION = $(shell cat VERSION)
else
VERSION = "$(shell git describe --abbrev=4 --dirty --always --tags)"
$(shell echo $(VERSION)-A > VERSION)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the "-A" suffix? assuming the one want's to contribute to synce4l, when he does make the VERSION file would be updated, shall he also include updated VERSION in a PR? Second thing if we have some kind of CI tests/scans, won't this be an issue? as running make will always modify the version even if the one updated the VERSION file explicitly, right? I mean something like: we want release version 1.0.0 but scans would be performed on version 1.0.0-A?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the "-A" suffix?

This is just a helper for the maintainer, on the release this would need to be manually changed to valid version. -A means it was generated automatically and is a reminder to fix that.

shall he also include updated VERSION in a PR?

No - I don't expect nobody to update the version but us. That is why I considered to leave it as "last version commit with -A or +" meaning it's "somewhere after after 0.9.0" like the lower one here:

$ git describe --abbrev=4 --dirty --always --tags
0.9.0-2-g312f-dirty
$ git describe --tags --abbrev=0
0.9.0 # To be honest I will do a `-A` or `+` here as well 

we want release version 1.0.0 but scans would be performed on version 1.0.0-A

No, the scans will never be an issue - this VERSION file is only a fallback solution when git repository is not there. All the CI and stuff will still use robust "git describe" option. Also, the official releases would have the same number updated in VERSION, which would not be -A.

I want to make any prefix to automatically added to explicitly see that the version number was generated automatically.. It's more like a reminder and helper for myself or you when I'm about to release official tag. I don't think it can hurt us in any way, but forgetting to update VERSION manually can.

I don't have strong feelings about this one, I can leave it for fully manual updates and write that in release procedure.
We can also see if any of the git local hooks can update the VERSION file for us automatically to always have the meaningful version + hash in place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the "-A" suffix?

This is just a helper for the maintainer, on the release this would need to be manually changed to valid version. -A means it was generated automatically and is a reminder to fix that.

shall he also include updated VERSION in a PR?

No - I don't expect nobody to update the version but us. That is why I considered to leave it as "last version commit with -A or +" meaning it's "somewhere after after 0.9.0" like the lower one here:

$ git describe --abbrev=4 --dirty --always --tags
0.9.0-2-g312f-dirty
$ git describe --tags --abbrev=0
0.9.0 # To be honest I will do a `-A` or `+` here as well 

we want release version 1.0.0 but scans would be performed on version 1.0.0-A

No, the scans will never be an issue - this VERSION file is only a fallback solution when git repository is not there. All the CI and stuff will still use robust "git describe" option. Also, the official releases would have the same number updated in VERSION, which would not be -A.

Ok, but isn't this code always add a "-A" to the version file when the folder contains '.git'?

I want to make any prefix to automatically added to explicitly see that the version number was generated automatically.. It's more like a reminder and helper for myself or you when I'm about to release official tag. I don't think it can hurt us in any way, but forgetting to update VERSION manually can.

I don't have strong feelings about this one, I can leave it for fully manual updates and write that in release procedure. We can also see if any of the git local hooks can update the VERSION file for us automatically to always have the meaningful version + hash in place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it would be adding it every time, but it won't be using it (the version with -A for VERSION in binary till the .git is there. Nor you don't have to commit that.

On the second thought, I will remove the overwriting of VERSION file since it seems not very clear and causes discussions - just to avoid confusions let me get rid of that and we will do our best to remember to update that manually.

Copy link
Contributor Author

@mmichaliINTC mmichaliINTC Jun 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhh - I've just double-checked, not only "Release" archives does not have the git repositoriy but also for example "download ZIP" from any branch won't have it. Example: you download the dev branch ZIP and you would have 0.9.0 version (so last Release tag) which:

  1. is not true,
  2. is very confusing.

I need to revisit the git post-commit hooks and see if it can update the version for us automatically every time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other projects like linuxptp (richardcochran/linuxptp@c686a86) are also fallbacking to hardcoded values, so I think we are good as well. Please see the updated code @kubalewski .

In the archive with code automatically generated by GitHub the
repository files are missing, so the VERSION cannot be created basing
on the commits history and tags.

Add a static VERSION file to enable compilation of code without the
access to git repository.

Signed-off-by: Michal Michalik <michal.michalik@intel.com>
@mmichaliINTC mmichaliINTC merged commit c568208 into dev Jul 3, 2023
@mmichaliINTC mmichaliINTC deleted the fix_version branch July 3, 2023 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants