Skip to content

Build: Support overwrite maven repo by gradle properties#2574

Merged
rdblue merged 2 commits intoapache:masterfrom
pan3793:gradle
May 20, 2021
Merged

Build: Support overwrite maven repo by gradle properties#2574
rdblue merged 2 commits intoapache:masterfrom
pan3793:gradle

Conversation

@pan3793
Copy link
Member

@pan3793 pan3793 commented May 10, 2021

def apacheReleasesRepoUrl = 'https://repository.apache.org/service/local/staging/deploy/maven2'
def snapshotsRepoUrl = project.hasProperty('mavenSnapshotsRepo') ? project.property('mavenSnapshotsRepo') : "$apacheSnapshotsRepoUrl"
def releasesRepoUrl = project.hasProperty('mavenReleasesRepo') ? project.property('mavenReleasesRepo') : "$apacheReleasesRepoUrl"
def snapshotsRepoUrl = project.hasProperty('mavenSnapshotsRepo') ? "$mavenSnapshotsRepo" : "$apacheSnapshotsRepoUrl"
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little lost here, it looks like previously we could set this based on the -PmavenSnapshotsRepo and now we are switching it to a local property? Or does -P handle set the local property as well?

Mostly i'm confused why project.property was not sufficient before

Copy link
Member

Choose a reason for hiding this comment

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

Groovy and Gradle still hold a lot of mysteries for me

Copy link
Member Author

Choose a reason for hiding this comment

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

Overwrite snapshotsRepo by -PmavenSnapshotsRepo? Seems not work for me.

As my understand, before only handle mavenUser and mavenPassword by -P or local properties but hardcode repo urls.

Groovy and Gradle still hold a lot of mysteries for me

Big +1

Copy link
Member

Choose a reason for hiding this comment

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

From my understanding project.property('mavenSnapshotsRepo') picks up properties from either a gradles.properties file or from -P args, I"ll run some tests to see

Copy link
Member Author

Choose a reason for hiding this comment

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

yeap, and -P args should have the high priority.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, i see. the inlined conversation diff looks weird, but File Changed page looks good.

Copy link
Member

Choose a reason for hiding this comment

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

I'm still lost a bit I think

  def snapshotsRepoUrl = project.hasProperty('mavenSnapshotsRepo') ? "$mavenSnapshotsRepo" : "$apacheSnapshotsRepoUrl"

should be

  def snapshotsRepoUrl = project.hasProperty('mavenSnapshotsRepo') ? project.property("mavenSnapshotsRepo") : "$apacheSnapshotsRepoUrl"

When I do a quick test

print(project.hasProperty("mavenSnapshotsRepo"))
print(project.property("mavenSnapshotsRepo"))
print($mavenSnapshotsRepo)

and

 g -PmavenSnapshotsRepo=one

Then I get mavenSnapshotsRepo undefined but the first two work fine.

Copy link
Member Author

@pan3793 pan3793 May 10, 2021

Choose a reason for hiding this comment

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

print("$mavenSnapshotsRepo") works but print($mavenSnapshotsRepo) not.

"$mavenSnapshotsRepo" will fallback to project.property("mavenSnapshotsRepo")

Copy link
Member

Choose a reason for hiding this comment

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

So this is some magic in the string interpolation?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

@RussellSpitzer
Copy link
Member

@aokolnychyi + @rdblue I haven't done a release so one of you should probably check it too :)

Copy link
Contributor

@rdblue rdblue left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

@rdblue rdblue merged commit 251e8a4 into apache:master May 20, 2021
@pan3793 pan3793 deleted the gradle branch February 18, 2022 01:40
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.

3 participants