You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2023/01/18 15:28:56 UTC
[GitHub] [lucene] magibney opened a new pull request, #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
magibney opened a new pull request, #12094:
URL: https://github.com/apache/lucene/pull/12094
buildAndPushRelease (release script) currently sets the username portion of the `ImplementationVersion` property MANIFEST.MF entry for built jars according the local machine username of the active user. It is straightforward to support explicitly setting this value, allowing for official Apache release artifacts to consistently indicate the apache Id of the release manager.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on a diff in pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
Posted by GitBox <gi...@apache.org>.
uschindler commented on code in PR #12094:
URL: https://github.com/apache/lucene/pull/12094#discussion_r1080679818
##########
dev-tools/scripts/buildAndPushRelease.py:
##########
@@ -120,6 +120,8 @@ def prepare(root, version, gpg_key_id, gpg_password, gpg_home=None, sign_gradle=
print(' prepare-release')
cmd = './gradlew --no-daemon assembleRelease' \
' -Dversion.release=%s' % version
+ if mf_username is not None:
+ cmd += ' -Dmanifest.username=%s' % mf_username
Review Comment:
This should be `-Pmanifest.username=%s`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #12094:
URL: https://github.com/apache/lucene/pull/12094#issuecomment-1396942514
I have also witnessed harassment from solr users towards the person whose name happens to be in there. Please, lets remove the username.
If I am ignored and this option is kept, I will use the option to populate something extremely offensive into the field rather than my real username, and create a release candidate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on a diff in pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
Posted by GitBox <gi...@apache.org>.
uschindler commented on code in PR #12094:
URL: https://github.com/apache/lucene/pull/12094#discussion_r1080678559
##########
gradle/java/jar-manifest.gradle:
##########
@@ -46,7 +46,9 @@ subprojects {
if (snapshotBuild) {
return "${project.version} ${gitRev} [snapshot build, details omitted]"
} else {
- return "${project.version} ${gitRev} - ${System.properties['user.name']} - ${buildDate} ${buildTime}"
+ def sysProps = System.properties
Review Comment:
Please don't use system properties directly for build properties; with gradle it should be project properties. Our build system has a method to get project properties which also falls back to sysprops. In short: use `propertyOrDefault('manifest.username', System.properties['user.name'])`
"user.name" is a real system property, so it is correct to use it here (otherwise you could fake it). But the project property should be given by gradle. This also allows to set it in your local gradle.properties.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on a diff in pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
Posted by GitBox <gi...@apache.org>.
uschindler commented on code in PR #12094:
URL: https://github.com/apache/lucene/pull/12094#discussion_r1080678559
##########
gradle/java/jar-manifest.gradle:
##########
@@ -46,7 +46,9 @@ subprojects {
if (snapshotBuild) {
return "${project.version} ${gitRev} [snapshot build, details omitted]"
} else {
- return "${project.version} ${gitRev} - ${System.properties['user.name']} - ${buildDate} ${buildTime}"
+ def sysProps = System.properties
Review Comment:
Please don't use system properties directly, with gradle it should be project propreties. Our build system has a method to get project properties which also falls back to sysprops. In short: use `propertyOrDefault('manifest.username', System.properties['user.name'])`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] magibney closed pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
Posted by "magibney (via GitHub)" <gi...@apache.org>.
magibney closed pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
URL: https://github.com/apache/lucene/pull/12094
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] magibney commented on pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
Posted by GitBox <gi...@apache.org>.
magibney commented on PR #12094:
URL: https://github.com/apache/lucene/pull/12094#issuecomment-1397358626
> Should we just remove the username from the manifest? This doesn't make sense to me, we don't put usernames anywhere else (e.g. no @author at apache)...
This seems fine to me. The RM is associated with the release via the GPG signature, which really is the only meaningful association with the release. That would substantially change the nature of this PR though. If we go that route I'll close this and open a new PR for removing the username from manifest? Is any further discussion necessary to go ahead with removing the username from the manifest?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #12094:
URL: https://github.com/apache/lucene/pull/12094#issuecomment-1396882440
Should we just remove the username from the manifest? This doesn't make sense to me, we don't put usernames anywhere else (e.g. no `@author` at apache)...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
Posted by GitBox <gi...@apache.org>.
uschindler commented on PR #12094:
URL: https://github.com/apache/lucene/pull/12094#issuecomment-1397386789
I am fine with both PRs, both technically correct. I don't care about username. If I would do a relaese I would insert "policeman" into the artifacts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] magibney commented on a diff in pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
Posted by GitBox <gi...@apache.org>.
magibney commented on code in PR #12094:
URL: https://github.com/apache/lucene/pull/12094#discussion_r1081614175
##########
gradle/java/jar-manifest.gradle:
##########
@@ -46,7 +46,9 @@ subprojects {
if (snapshotBuild) {
return "${project.version} ${gitRev} [snapshot build, details omitted]"
} else {
- return "${project.version} ${gitRev} - ${System.properties['user.name']} - ${buildDate} ${buildTime}"
+ def sysProps = System.properties
Review Comment:
I've adjusted this PR accordingly, but following on Robert's feedback also opened a PR that simply removes the username from MANIFEST.MF. Assuming we want to go that direction, we can just close this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on a diff in pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
Posted by GitBox <gi...@apache.org>.
uschindler commented on code in PR #12094:
URL: https://github.com/apache/lucene/pull/12094#discussion_r1080680435
##########
dev-tools/scripts/buildAndPushRelease.py:
##########
@@ -120,6 +120,8 @@ def prepare(root, version, gpg_key_id, gpg_password, gpg_home=None, sign_gradle=
print(' prepare-release')
cmd = './gradlew --no-daemon assembleRelease' \
' -Dversion.release=%s' % version
Review Comment:
Actually this should also be `-P`, but it won't break, but for consistency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] magibney commented on pull request #12094: releaseWizard: allow explicitly setting MANIFEST.MF userid (e.g., to apache id)
Posted by "magibney (via GitHub)" <gi...@apache.org>.
magibney commented on PR #12094:
URL: https://github.com/apache/lucene/pull/12094#issuecomment-1401040344
Closed; superseded by https://github.com/apache/lucene/pull/12096
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org