Skip to content

Conversation

@jasontedor
Copy link
Member

When finding the commit hash for the build to place in the JAR manifest (which is used to identity the build), the scm-info plugin assumes that GIT_COMMIT is the commit for this build. That assumption is wrong, this build could be a sub-build of another build that GIT_COMMIT belongs to. If GIT_COMMIT is set, we ignore the commit hash calculated by scm-info and calculate the hash ourselves.

When finding the commit hash for the build to place in the JAR manifest
(which is used to identity the build), the scm-info plugin assumes that
GIT_COMMIT is the commit for this build. That assumption is wrong, this
build could be a sub-build of another build that GIT_COMMIT belongs
to. If GIT_COMMIT is set, we ignore the commit hash calculated by
scm-info and calculate the hash ourselves.
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

* is the commit hash for that build. Therefore, if GIT_COMMIT is set we calculate the commit hash ourselves.
*/
if (System.getenv("GIT_COMMIT") != null) {
def hash = new RepositoryBuilder().findGitDir(project.buildDir).build().resolve(Constants.HEAD).name
Copy link
Member

Choose a reason for hiding this comment

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

String instead of def please?

@jasontedor jasontedor merged commit 4656c55 into elastic:master Jan 4, 2018
jasontedor added a commit that referenced this pull request Jan 4, 2018
When finding the commit hash for the build to place in the JAR manifest
(which is used to identity the build), the scm-info plugin assumes that
GIT_COMMIT is the commit for this build. That assumption is wrong, this
build could be a sub-build of another build that GIT_COMMIT belongs
to. If GIT_COMMIT is set, we ignore the commit hash calculated by
scm-info and calculate the hash ourselves.

Relates #28082
jasontedor added a commit that referenced this pull request Jan 4, 2018
When finding the commit hash for the build to place in the JAR manifest
(which is used to identity the build), the scm-info plugin assumes that
GIT_COMMIT is the commit for this build. That assumption is wrong, this
build could be a sub-build of another build that GIT_COMMIT belongs
to. If GIT_COMMIT is set, we ignore the commit hash calculated by
scm-info and calculate the hash ourselves.

Relates #28082
jasontedor added a commit that referenced this pull request Jan 4, 2018
When finding the commit hash for the build to place in the JAR manifest
(which is used to identity the build), the scm-info plugin assumes that
GIT_COMMIT is the commit for this build. That assumption is wrong, this
build could be a sub-build of another build that GIT_COMMIT belongs
to. If GIT_COMMIT is set, we ignore the commit hash calculated by
scm-info and calculate the hash ourselves.

Relates #28082
jasontedor added a commit that referenced this pull request Jan 4, 2018
When finding the commit hash for the build to place in the JAR manifest
(which is used to identity the build), the scm-info plugin assumes that
GIT_COMMIT is the commit for this build. That assumption is wrong, this
build could be a sub-build of another build that GIT_COMMIT belongs
to. If GIT_COMMIT is set, we ignore the commit hash calculated by
scm-info and calculate the hash ourselves.

Relates #28082
@jasontedor jasontedor deleted the ignore-git-commit branch January 4, 2018 20:00
@mhujer
Copy link

mhujer commented Jan 4, 2018

Is it possible that something went wrong with this patch? If I download build snapshot from https://download.elastic.co/esvm/snapshots/6.1.zip it contains esCommi as a build_hash - see bellow:

{
  "name" : "q9gSC4_",
  "cluster_name" : "elasticsearch",
  "cluster_uuid" : "zNOfg0gQTwC0UFDoVjEcpg",
  "version" : {
    "number" : "6.1.2",
    "build_hash" : "esCommi",
    "build_date" : "2018-01-04T18:42:14.730Z",
    "build_snapshot" : true,
    "lucene_version" : "7.1.0",
    "minimum_wire_compatibility_version" : "5.6.0",
    "minimum_index_compatibility_version" : "5.0.0"
  },
  "tagline" : "You Know, for Search"
}

// Related to elastic/elasticsearch-php#711

@jasontedor
Copy link
Member Author

There's no chance that was caused by this change. It was caused by this change to the esvm snapshot builder. I've reached out internally to have it fixed.

spalger added a commit to elastic/esvm-snapshot-builder that referenced this pull request Jan 4, 2018
Remove GIT_COMMIT setting, since it didn't work and elastic/elasticsearch#28082 is merged
@spalger
Copy link
Contributor

spalger commented Jan 4, 2018

@mhujer Sorry about the hasty fix/bug, https://download.elastic.co/esvm/snapshots/6.1.zip is correct now:

{
  "name" : "Ldm-0Ow",
  "cluster_name" : "elasticsearch",
  "cluster_uuid" : "gsH7uboEQWKUrmK43z_nBQ",
  "version" : {
    "number" : "6.1.2",
    "build_hash" : "e345797",
    "build_date" : "2018-01-04T20:58:28.717Z",
    "build_snapshot" : true,
    "lucene_version" : "7.1.0",
    "minimum_wire_compatibility_version" : "5.6.0",
    "minimum_index_compatibility_version" : "5.0.0"
  },
  "tagline" : "You Know, for Search"
}

@mhujer
Copy link

mhujer commented Jan 5, 2018

Thanks for fixing it! 👍

@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team v5.6.6 v6.0.2 v6.1.2 v6.2.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants