-
-
Notifications
You must be signed in to change notification settings - Fork 742
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
ICU-22601 v74.2 fix dangling LICENSE file #2749
ICU-22601 v74.2 fix dangling LICENSE file #2749
Conversation
bd97ef0
to
b4e4ca0
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
- 'git archive' works on a subtree so did not include LICENSE - we copy the LICENSE file from the build dir - broken by ICU-22309
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.
This looks right to me. Thanks, Steven!
please go ahead and merge once the ticket's accepted |
Thanks @srl295 for figuring this out and fixing it! We normally fix bugs on |
PS: If we don't roll yet another 74.x maintenance release, then we don't really need this fix on the maint/maint-74 branch at all. |
Sure, I'll rebase and post it to Also pragmatically, it could be copied to a local workspace (such as on top of |
b4e4ca0
to
dc9a6af
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
@sven-oly @markusicu PTAL |
I should add that i would recommend re-releasing the source files with a LICENSE. Putting my license hat on.. |
Makes sense. I asked Craig to try this. I think he normally runs this from Docker, but since we only need to update the source archives, that should not be necessary.
Yes, we will update the source archives on the release page. |
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.
thanks!
Problems: I've tried this by manually updating the dist.mk file in the downloaded ICU 74.2 source under
File icu4c-74_2-src.tgz has only a link for LICENSE, but no file. icu4c-74_2-src.zip has no LICENSE file at all. What am I missing? |
Sure. It's also just uninstallable without it for us ;) |
This after deleting those files?
“cp -fv $(ICU4CTOP)/LICENSE "$(DISTY_TMP)/LICENSE" did this end up copying
a file ? Should appear in logs. Can you paste the logs?
|
That too!! |
you should be able to just checkout this branch directly and build it under icu-docker. I'll try that also. It'll build main but it should then build correctly. |
@sven-oly: I
$ tar tvfpz dist/*-src.d/icu4c-src.tgz | grep LICENSE
-rw-r--r-- build/build 25185 2023-12-19 21:35 icu/LICENSE
$ unzip -l dist/*-src.d/icu4c-src.zip | grep LICENSE
25697 2023-12-19 21:35 icu/LICENSE |
@markusicu @sven-oly this is working for me locally and via docker as shown, should we merge this? |
ping? |
ICU-22601
Checklist
broken by ICU-22309