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

Get project version from git #5403

Merged
merged 7 commits into from
Dec 12, 2018

Conversation

inodb
Copy link
Member

@inodb inodb commented Dec 6, 2018

Normally for each new release we have to create a PR just to update the version in the pom. Then afterwards we get conflicts when we try n merge master branch back into feature branches. Imagine updating master branch, now you will get conflicts in release-x branch and rc branch.

Maven should not keep track of the versioning at all, this is already managed by git. This maven-external-version plugin determines the version using a bash script, version.sh. This tries to get the version from git. If git is not available at compile time, one can also supply the version by setting an env variable PROJECT_VERSION instead.

I slightly modified maven-external-version plugin, so we can download that dependency from jitpack.

Unfortunately ${project.version} doesn't work in plugins, so we can't show the
version name in the actual war file name anymore (this is a known issue:
bdemers/maven-external-version#7). Seems like an ok
trade-off though

Another issue I ran into was that integration-test didn't work anymore without running install first. Not sure if this is a reason for concern, but just wanted to notify @ersinciftci . There might be a way to update the plugin to avoid having to do this, but maybe it's not that big of a deal.

@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from 20091d2 to 8a56287 Compare December 6, 2018 17:25
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from 8a56287 to 79c6343 Compare December 6, 2018 21:06
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from d9cde51 to 9388802 Compare December 6, 2018 21:31
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from 9388802 to 51b5fb7 Compare December 6, 2018 21:36
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from 51b5fb7 to f7c1c6b Compare December 6, 2018 21:41
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from f7c1c6b to b49bc64 Compare December 6, 2018 21:46
@jjgao jjgao temporarily deployed to cbioportal-pr-5403 December 6, 2018 21:46 Inactive
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from b49bc64 to b3c04ee Compare December 6, 2018 21:54
@jjgao jjgao temporarily deployed to cbioportal-pr-5403 December 6, 2018 21:54 Inactive
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from b3c04ee to 51953ba Compare December 6, 2018 22:03
@jjgao jjgao temporarily deployed to cbioportal-pr-5403 December 6, 2018 22:03 Inactive
version.sh Show resolved Hide resolved
pom.xml Show resolved Hide resolved
- Update dependency versions. ${project.version} in dependencies weren't
  updated.
- Update plugins' dependency versions. Those weren't updated either.
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from 58a4b9a to d07f8cc Compare December 7, 2018 23:59
@jjgao jjgao temporarily deployed to cbioportal-pr-5403 December 8, 2018 00:00 Inactive
@jjgao jjgao temporarily deployed to cbioportal-pr-5403 December 8, 2018 00:14 Inactive
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from cd4731a to 29a5968 Compare December 8, 2018 00:19
@jjgao jjgao temporarily deployed to cbioportal-pr-5403 December 8, 2018 00:19 Inactive
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from 29a5968 to bf1af8b Compare December 8, 2018 00:38
@jjgao jjgao temporarily deployed to cbioportal-pr-5403 December 8, 2018 00:38 Inactive
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from bf1af8b to dfce488 Compare December 10, 2018 18:03
@jjgao jjgao temporarily deployed to cbioportal-pr-5403 December 10, 2018 18:03 Inactive
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from dfce488 to d09b4a9 Compare December 10, 2018 18:07
@jjgao jjgao temporarily deployed to cbioportal-pr-5403 December 10, 2018 18:08 Inactive
- update mvn version
- run install before integration-test when doing core tests (otherwise the
  version is not correctly set, there might be a way to update
  maven-external-version to avoid this)
@inodb inodb force-pushed the generate-pom-version-from-git-commit branch from d09b4a9 to 45456d4 Compare December 10, 2018 19:09
@jjgao jjgao temporarily deployed to cbioportal-pr-5403 December 10, 2018 19:10 Inactive
Copy link
Contributor

@sheridancbio sheridancbio left a comment

Choose a reason for hiding this comment

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

looks good, although I recommend a (manual) test where connected changes in two modules in two different changesets still are able to find each other:

  1. create new branch
  2. modify some service layer function to take one additional argument .. commit
  3. modify related web layer handlers to call the modified service layer function with additional arg .. commit
  4. try to compile and see if tests pass

# fetch all tags silently
git fetch --tags --quiet > /dev/null 2> /dev/null
# get human readable commit id from git
git describe --tags --always --dirty | sed 's/^v//'
Copy link
Contributor

Choose a reason for hiding this comment

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

If git describe always goes back to a tag, does it mean that portal dependencies can only be brought in from a tagged commit that is pushed to the cbioportal github repo? If a single PR makes changes in both the service and web layer (like changing the service interface), will the "new" web layer be able to bring in the "new" service layer even if a tag has not been created (in a pr to rc, for instance)? Maybe the extra info after the tag is enough to provide the additional path since the tag (the "dirty" part I guess?)

% git describe --tags --always --dirty

v1.18.0-120-g05c7ad6-dirty

Copy link
Member Author

Choose a reason for hiding this comment

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

@sheridancbio Portal dependencies can be brought in locally or remotely. It doesn't need to be tagged/committed in the github repo. Maven continues to function in the same way, so if u compile something locally the artifacts will end up in your ~/.m2 folder and you can include them in another project as a dependency.

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good. I was mainly thinking about dependencies between the modules within cbioportal, but packaging the cbioportal code into other codebases is also needed.

@jjgao jjgao temporarily deployed to cbioportal-pr-5403 December 11, 2018 23:34 Inactive
@inodb
Copy link
Member Author

inodb commented Dec 12, 2018

I found another issue when including cbioportal through jitpack as a dependency as we do with e.g. genome-nexus-annotation-pipeline. The scripts pom would not get the updated parent project's version. Not generating a new pom by the shaded maven plugin solved that issue

@inodb inodb merged commit c6df35b into cBioPortal:master Dec 12, 2018
@fedde-s
Copy link
Member

fedde-s commented Dec 12, 2018

@inodb: what do you mean? Are you no longer generating the scripts JAR? It's required for all data loading.

@fedde-s
Copy link
Member

fedde-s commented Dec 12, 2018

So if that's the case, we have to revert the merge right away.

inodb added a commit to inodb/genome-nexus that referenced this pull request Apr 8, 2020
It's a hassle to keep updating the pom.xml versions. Similar to
cBioPortal/cbioportal we would like to tag the github repo and have the
maven project version update automatically. See also:
cBioPortal/cbioportal#5403
inodb added a commit to inodb/genome-nexus that referenced this pull request Apr 8, 2020
It's a hassle to keep updating the pom.xml versions. Similar to
cBioPortal/cbioportal we would like to tag the github repo and have the
maven project version update automatically. See also:
cBioPortal/cbioportal#5403
inodb added a commit to inodb/genome-nexus-annotation-pipeline that referenced this pull request Feb 23, 2023
This updates the pom version based on the git version/commit. This is
the same as it works for cBioPortal:
cBioPortal/cbioportal#5403.
inodb added a commit to inodb/genome-nexus-annotation-pipeline that referenced this pull request Feb 23, 2023
This updates the pom version based on the git version/commit. This is
the same as it works for cBioPortal:
cBioPortal/cbioportal#5403.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants