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

ICU-22855: Create combined source distribution when running make dist on C++ side #3097

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

catamorphism
Copy link
Contributor

@catamorphism catamorphism commented Aug 12, 2024

This PR changes make dist on the C++ side to create a single combined sources distribution, including icu4c/, icu4j/, and testdata/. It also undoes the change in intltest.cpp that looks for the testdata directory in multiple locations.

Note: I haven't changed anything on the Java side because I'm not sure what the layout of the .jar file should look like.

Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22855
  • Required: The PR title must be prefixed with a JIRA Issue number.
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number.
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

@catamorphism
Copy link
Contributor Author

@markusicu Is this something like what you had in mind? Also cc @srl295

@@ -65,25 +65,25 @@ $(DISTY_FILE_TGZ) $(DISTY_FILE_ZIP) $(DISTY_DATA_ZIP): $(DISTY_DAT) $(DISTY_TMP
@echo Export icu4c@$(GITVER) to "$(DISTY_TMP)/icu"
-$(RMV) $(DISTY_FILE) $(DISTY_TMP)
$(MKINSTALLDIRS) $(DISTY_TMP)
( cd $(ICU4CTOP)/.. && git archive --format=tar --prefix=icu/ HEAD:icu4c/ ) | ( cd "$(DISTY_TMP)" && tar xf - )
( cd $(ICU4CTOP)/.. && git archive --format=tar --prefix=icu/ HEAD -- icu4c/ icu4j/ testdata/ ) | ( cd "$(DISTY_TMP)" && tar xf - )
Copy link
Member

Choose a reason for hiding this comment

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

probably top level README* LICENSE*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added LICENSE and README

# special handling for LICENSE file. The symlinks will be included as files by tar and zip.
cp -fv $(ICU4CTOP)/LICENSE "$(DISTY_TMP)/LICENSE"
# Copy top-level testdata directory so it's a sibling of the source/ directory
cp -fv $(ICU4CTOP)/LICENSE "$(DISTY_ICU)/LICENSE"
Copy link
Member

Choose a reason for hiding this comment

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

may need revisit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this line due to the other change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants