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

chore(docker): Use the output of printSemVersion as ORT_VERSION #7622

Closed
wants to merge 1 commit into from

Conversation

fviernau
Copy link
Member

@fviernau fviernau commented Oct 4, 2023

Previously, the value looked like 3.0.0-SNAPSHOT. Change it to the form like 3.0.0-389da0c-SNAPSHOT, so that it is possible to figure out from which sources ORT has been built.

Previously, the value looked like 3.0.0-SNAPSHOT. Change it to the form
like `3.0.0-389da0c-SNAPSHOT`, so that it is possible to figure out from
which sources ORT has been built.

Signed-off-by: Frank Viernau <[email protected]>
@fviernau fviernau requested a review from a team as a code owner October 4, 2023 08:49
@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (13a4714) 68.03% compared to head (e8a7fab) 68.03%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #7622   +/-   ##
=========================================
  Coverage     68.03%   68.03%           
  Complexity     2022     2022           
=========================================
  Files           344      344           
  Lines         16725    16725           
  Branches       2371     2371           
=========================================
  Hits          11379    11379           
  Misses         4363     4363           
  Partials        983      983           
Flag Coverage Δ
funTest-docker 69.36% <ø> (ø)
funTest-non-docker 36.44% <ø> (ø)
test 35.55% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -169,7 +169,7 @@ jobs:

- name: Get ORT current version
run: |
ORT_VERSION=$(./gradlew -q properties --property version | sed -nr "s/version: (.+)/\1/p")
ORT_VERSION=$(./gradlew -q printSemVersion)
Copy link
Member

Choose a reason for hiding this comment

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

Note that when being exactly on tag 2.0.0 (with a clean working tree), this would print 2.0.0+sha.5893bd7 instead of 2.0.0. Is this desired?

Copy link
Member Author

@fviernau fviernau Oct 4, 2023

Choose a reason for hiding this comment

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

I wasn't aware of that, but I think it's fine. I'd be fine if this logic was improved later on to only print 2.0.0 in that case. Do you agree?

Copy link
Member

Choose a reason for hiding this comment

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

That would be a mismatch, however, for people running ORT also from the distribution archive, which shows 2.0.0 in this case. I'll look into customizing the version format to address that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Shall we merge now this and then customize the version format in that gradle tasks later on?

Copy link
Member

@sschuberth sschuberth Oct 4, 2023

Choose a reason for hiding this comment

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

My alternative proposal is already here. That could be used instead of this PR (not in addition to it). The advantage is that also non-Docker user would get the SHA1 info for pre-release builds.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can minimize it direct in the tagging, i can capture the sha without need to alter the current Gradle process.
But will solve only for the images in upstream, not the local build script/.

@fviernau
Copy link
Member Author

fviernau commented Oct 4, 2023

Close in favor of #7628 .

@fviernau fviernau closed this Oct 4, 2023
@sschuberth sschuberth deleted the docker-ort-version-string branch October 4, 2023 14:13
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.

3 participants