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

Build: Move gradle wrapper jar to a dot dir #30146

Merged
merged 3 commits into from
May 1, 2018

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Apr 25, 2018

This commit moves the gradle wrapper jar file to a hidden directory, so
that it does not clutter the top level names seen when doing an ls in
the project. The actual jar file is never manually edited, and only
changed by running ./gradlew wrapper ... so it is not important for
this directory to be "visible".

This commit moves the gradle wrapper jar file to a hidden directory, so
that it does not clutter the top level names seen when doing an ls in
the project. The actual jar file is never manually edited, and only
changed by running `./gradlew wrapper ...` so it is not important for
this directory to be "visible".
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

Fine by me.

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

This does not appear to work. I checked out your PR, I execute

$ ./gradlew wrapper --gradle-version=4.6

I end up with:

16:52:32 [jason:~/src/elastic/elasticsearch] pr/30146(+0/-0)+ ± cat .gradle-wrapper/gradle-wrapper.properties 
distributionSha256Sum=9af7345c199f1731c187c96d3fe3d31f5405192a42046bafa71d846c3d9adacb

Also:

16:53:03 [jason:~/src/elastic/elasticsearch] pr/30146(+0/-0)+ ± git diff --cached
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
modified: .gradle-wrapper/gradle-wrapper.properties
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ gradle-wrapper.properties:1 @
distributionUrl=https\://services.gradle.org/distributions/gradle-4.7-all.zip
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStorePath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
distributionSha256Sum=203f4537da8b8075e38c036a6d14cb71b1149de5bf0a8f6db32ac2833a1d1294
distributionSha256Sum=9af7345c199f1731c187c96d3fe3d31f5405192a42046bafa71d846c3d9adacb

I am also concerned about the fact that you had to manually edit the wrapper scripts. I would prefer those to come along with the Gradle wrapper task execution and it appears it does not.

This feels like a hack to me. I would prefer that this be managed upstream and we benefit from it.

@rjernst
Copy link
Member Author

rjernst commented Apr 25, 2018

I am also concerned about the fact that you had to manually edit the wrapper scripts.

I did not edit the wrapper scripts. I ran the gradle wrapper task. The diff you see was a side effect of broken runs as I changed things, and ended up doing a git mv of the properties file. I've done the regex again and updated here.

@rjernst
Copy link
Member Author

rjernst commented Apr 25, 2018

Hrm, I do get an error now a second time running the wrapper task. I will look into this more.

@jasontedor
Copy link
Member

I am confused, this is still broken.

@jasontedor
Copy link
Member

Okay, race condition on that last comment. Good luck.

@rjernst
Copy link
Member Author

rjernst commented Apr 25, 2018

Ok this is all fixed with 20cc208. I was wiping out the properties file when putting in the checksum.

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

@rjernst rjernst merged commit a324cd4 into elastic:master May 1, 2018
@rjernst rjernst deleted the hide_gradle_wrapper branch May 1, 2018 15:47
rjernst added a commit that referenced this pull request May 1, 2018
This commit moves the gradle wrapper jar file to a hidden directory, so
that it does not clutter the top level names seen when doing an ls in
the project. The actual jar file is never manually edited, and only
changed by running `./gradlew wrapper ...` so it is not important for
this directory to be "visible".
rjernst added a commit that referenced this pull request May 1, 2018
This reverts commit a324cd4.

A non default gradle wrapper location causes intellij to fail in
mysterious ways....
rjernst added a commit that referenced this pull request May 1, 2018
This reverts commit a324cd4.

A non default gradle wrapper location causes intellij to fail in
mysterious ways....
dnhatn added a commit that referenced this pull request May 2, 2018
* master: (68 commits)
  [DOCS] Removes X-Pack Elasticsearch release notes (#30272)
  Correct an example in the top-level suggester documentation. (#30224)
  [DOCS] Removes broken link
  [DOCS] Adds file realm configuration details (#30221)
  [DOCS] Adds PKI realm configuration details (#30225)
  Fix a reference to match_phrase_prefix in the match query docs. (#30282)
  Fix failure for validate API on a terms query (#29483)
  [DOCS] Fix 6.4-specific link in changelog (#30314)
  Remove RepositoriesMetaData variadic constructor (#29569)
  Test: increase authentication logging for debugging
  [DOCS] Removes redundant SAML realm settings (#30196)
  REST Client: Add Request object flavored methods (#29623)
  [DOCS] Adds changelog to Elasticsearch Reference (#30271)
  [DOCS] Fixes section error
  SQL: Teach the CLI to ignore empty commands (#30265)
  [DOCS] Adds Active Directory realm configuration details (#30223)
  [DOCS] Removes redundant file realm settings (#30192)
  [DOCS] Fixes users command name (#30275)
  Build: Move gradle wrapper jar to a dot dir (#30146)
  Build: Log a warning if disabling reindex-from-old (#30304)
dnhatn added a commit that referenced this pull request May 2, 2018
* 6.x: (62 commits)
  [DOCS] Adds new installation package details (#29590)
  Revert "Build: Move gradle wrapper jar to a dot dir (#30146)"
  [DOCS] Added 6.3.0 section to changelog
  [DOCS] Merge 6.x release notes into changelog (#30312)
  [DOCS] Removes broken link
  [DOCS] Adds file realm configuration details (#30221)
  [DOCS] Adds PKI realm configuration details (#30225)
  [DOCS] Fix 6.4-specific link in changelog (#30314)
  Remove RepositoriesMetaData variadic constructor (#29569)
  [DOCS] Adds changelog to Elasticsearch Reference (#30310)
  Test: increase authentication logging for debugging
  [DOCS] Removes redundant SAML realm settings (#30196)
  SQL: Teach the CLI to ignore empty commands (#30265)
  [DOCS] Fixes section error
  [DOCS] Adds Active Directory realm configuration details (#30223)
  [DOCS] Removes redundant file realm settings (#30192)
  [DOCS] Fixes users command name (#30275)
  Build: Move gradle wrapper jar to a dot dir (#30146)
  Build: Log a warning if disabling reindex-from-old (#30304)
  TEST: Add debug log to FlushIT
  ...
jasontedor added a commit that referenced this pull request May 2, 2018
* master:
  Fix message content in users tool (#30293)
  [DOCS] Fixes links to breaking changes
  [DOCS] Adds new installation package details (#29590)
  Revert "Build: Move gradle wrapper jar to a dot dir (#30146)"
@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 >non-issue Team:Delivery Meta label for Delivery team v6.4.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants