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

proposed "develop" build versions #6699

Merged

Conversation

garyschulte
Copy link
Contributor

PR description

As part of our recent build and release changes, we no longer need to explicitly maintain the build version in gradle.properties file.

This PR is a proposal to change our "develop" builds to use git commit and HEAD details to specify a dynamic build version.

for example this PR, prior to committing the changes, produced a build named:
besu-24.3.7-develop-610927511e.tar.gz

After committing the gradle changes and rebuilding, it produced a build named:
besu-24.3.7-develop-600d812d89.tar.gz

Open to suggestions, feedback, and identifying corner cases

Fixed Issue(s)

build.gradle Outdated
}
return version
def gitDetails = getGitCommitDetails(10) // Adjust length as needed
return "${gitDetails.date}-develop-${gitDetails.hash}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jflo thinking about the impact of this on docker builds. we would probably have a build explosion if we published versions like this on dockerhub. Does the new CI pipeline explicitly push versions into snapshot builds on main ? or is that something we need to account for here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I disabled the duplicate circle-ci workflows in this PR that are causing us to publish docker images to dockerhub using snapshot build tags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted the change in favor of just disabling the circle-ci webhook

Copy link
Contributor

Choose a reason for hiding this comment

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

We no longer push interim builds on every push to main. Nobody uses them, so they were a waste of cycles and space. This should be safe.

Choose a reason for hiding this comment

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

hey I'm "nobody" 😂

Copy link
Contributor

Choose a reason for hiding this comment

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

you can still have build artifacts, just not docker images: https://github.com/hyperledger/besu/releases/tag/develop

@garyschulte garyschulte requested review from jflo and shemnon March 8, 2024 19:50
@garyschulte garyschulte force-pushed the feature/dynamic-build-version branch 2 times, most recently from ecb9e65 to e7e4185 Compare March 8, 2024 20:34
Copy link
Contributor

@jflo jflo left a comment

Choose a reason for hiding this comment

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

Love it, but we do need to still honor -Pversion when it is used by the release pipeline.

@@ -119,7 +119,7 @@ allprojects {
apply plugin: 'net.ltgt.errorprone'
apply from: "${rootDir}/gradle/versions.gradle"

version = rootProject.version
Copy link
Contributor

Choose a reason for hiding this comment

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

this would keep us from being able to inject the release version we want via -Pversion=${{github.ref_name}} so we need to only use calculateVersion if that isn't set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a provision for build version to be supplied 👍 . I also force it to be semver or semver-something

build.gradle Outdated
}
return version
def gitDetails = getGitCommitDetails(10) // Adjust length as needed
return "${gitDetails.date}-develop-${gitDetails.hash}"
Copy link
Contributor

Choose a reason for hiding this comment

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

We no longer push interim builds on every push to main. Nobody uses them, so they were a waste of cycles and space. This should be safe.

@garyschulte garyschulte force-pushed the feature/dynamic-build-version branch from 2b25b38 to dcce662 Compare March 13, 2024 16:57
…r than 24.3.13-develop-dcce662e8c

Signed-off-by: garyschulte <[email protected]>
@garyschulte garyschulte enabled auto-merge (squash) March 13, 2024 17:24
@garyschulte garyschulte merged commit 39a356f into hyperledger:main Mar 13, 2024
35 of 42 checks passed
@garyschulte garyschulte deleted the feature/dynamic-build-version branch March 13, 2024 22:11
amsmota pushed a commit to Citi/besu that referenced this pull request Apr 16, 2024
* proposed "develop" build versions

Signed-off-by: garyschulte <[email protected]>
Signed-off-by: amsmota <[email protected]>
amsmota pushed a commit to Citi/besu that referenced this pull request Apr 16, 2024
* proposed "develop" build versions

Signed-off-by: garyschulte <[email protected]>
Signed-off-by: amsmota <[email protected]>
matthew1001 pushed a commit to kaleido-io/besu that referenced this pull request Jun 7, 2024
* proposed "develop" build versions

Signed-off-by: garyschulte <[email protected]>
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