-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
LUCENE-10107 #2575
LUCENE-10107 #2575
Conversation
lucene/common-build.xml
Outdated
@@ -764,7 +764,7 @@ | |||
<!-- If possible, include the GIT hash into manifest: --> | |||
<exec dir="." executable="${git.exe}" outputproperty="-checkoutid" errorproperty="-giterr" failifexecutionfails="false"> | |||
<arg value="log"/> | |||
<arg value="--format=%H"/> | |||
<arg value="--format=%h"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it could also be a more verbose command like this one (note the extended abbreviated hash syntax):
git show --pretty=oneline --abbrev-commit --abbrev=15 --no-patch HEAD
1549321
to
6cb92e4
Compare
This worked for me on macOS but I'd like somebody else to try this with Windows or Linux before we merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried these changes out in the 8.10 RC build checkout I have locally and works well but on Mac too.
verifyRevision = 'Implementation-Version: %s %s' % (version, gitRevision) | ||
if s.find(verifyRevision) == -1: | ||
raise RuntimeError('%s is missing "%s" inside its META-INF/MANIFEST.MF (wrong git revision?)' % \ | ||
match = re.search("Implementation-Version: (.+\r\n .+)", s, re.MULTILINE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will the MANIFEST line endings always be CRLF
? According to the spec it could be CR LF | LF | CR
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if CRLF on my machine was a Mac setting or something that ant or java tooling was doing. Hence the request for Windows/Linux testing help. But I chatted with Tim and confirmed that at least his machine was also producing CRLF, so this wouldn't be holding up the release. I guess the correct expression would be (\r\n?|\r)␣
and then would have to do something more intelligent with the line removal too.
https://issues.apache.org/jira/browse/LUCENE-10107