Skip to content

ICU-21627 - Fix Makefile bugs#1732

Closed
pchemguy wants to merge 15 commits intounicode-org:mainfrom
pchemguy:maint/maint-68
Closed

ICU-21627 - Fix Makefile bugs#1732
pchemguy wants to merge 15 commits intounicode-org:mainfrom
pchemguy:maint/maint-68

Conversation

@pchemguy
Copy link
Copy Markdown

The two libraries are in the wrong order causing the static build to fail (at least on Windows). There are three more Makefile's (in tests and samples) with the same issue. However, those have no apparent effect on the build process (it completes successfully), so I did not fix them.

Static libraries should not use numeric suffixes on the MinGW/Windows platform. Additionally, the static "s" suffix should also be dropped.

https://unicode-org.atlassian.net/browse/ICU-21627

hugovdm and others added 15 commits December 9, 2020 13:43
Yoshito is making this change on behalf of @datalogics-robb.
IBM's XL compiler can only support c++11 when the 'clang front end' is used.
This change modifies the configure step to use xlclang and  xlclang++
Change readme.html reference to xlC to refer to xlclang++ instead.
…ic DST setting is OFF to ICU 68.

cherry-picked from commit a1a1faf
This change backports the GHA script to the maint/maint-68 branch.

Related ICU tickets:
https://unicode-org.atlassian.net/browse/ICU-21434
https://unicode-org.atlassian.net/browse/ICU-21450

Cherry-picked from: 8078ae4
Cherry-picked from: 38125f3
Cherry-picked from: e5a3f2b
Cherry-picked from: c7ea02f
The two libraries are in the wrong order causing the static build to fail (at least on Windows). There are three more Makefile's (in tests and samples) with the same issue. However, those have no apparent effect on the build process (it completes successfully), so I did not fix them.
Remove static and version suffixes on MinGW platform

Static libraries should not use numeric suffixes on the MinGW/Windows platform. Additionally, the static "s" suffix should also be dropped.
@markusicu markusicu requested a review from srl295 May 26, 2021 17:55
@markusicu markusicu changed the base branch from maint/maint-68 to main May 26, 2021 17:57
@jefgen
Copy link
Copy Markdown
Member

jefgen commented May 26, 2021

markusicu changed the base branch from maint/maint-68 to main

Ah, I think this might have caused git/GitHub to get confused, as now this PR shows 15 commits to be merged into main instead of just one.

I think the commit for this PR might need to be rebased onto main and then this PR could be forced pushed to be updated.

@jefgen
Copy link
Copy Markdown
Member

jefgen commented May 26, 2021

Thank you very much for creating this PR, signing the CLA, and also for filing a ticket! :)

FWIW, the changes in the commit here: 956683a seem mostly fine to me.

However, I'm not super familiar with the MinGW build environment.
There is another recent PR #1733, which I think will conflict with these changes though.

For the static library changes, that PR removes STATIC_PREFIX_WHEN_USED -- which perhaps does a similar thing to setting it to empty string I think (?).

Would you be able to take a look at the other PR?
It would be great to have people that are more familiar with MinGW, in order to help to review MinGW related the changes.

Thank you!

@pchemguy
Copy link
Copy Markdown
Author

pchemguy commented May 26, 2021

Thank you very much for creating this PR, signing the CLA, and also for filing a ticket! :)

FWIW, the changes in the commit here: 956683a seem mostly fine to me.

However, I'm not super familiar with the MinGW build environment.
There is another recent PR #1733, which I think will conflict with these changes though.

For the static library changes, that PR removes STATIC_PREFIX_WHEN_USED -- which perhaps does a similar thing to setting it to empty string I think (?).

Would you be able to take a look at the other PR?
It would be great to have people that are more familiar with MinGW, in order to help to review MinGW related the changes.

Thank you!

@jefgen
I can take my commit, replace source/config/mh-mingw with mh-mingw from "ICU-21628 MinGW/Windows incorrect build #1733" and it builds statically (Windows/MinGW).

#1733 does not remove STATIC_PREFIX_WHEN_USED, because it was not there at all. I added STATIC_PREFIX_WHEN_USED and STATIC_PREFIX to mh-mingw, but #1733 is based on commit not incorporating mine. #1733 achieves the same result by removing STATIC_PREFIX from LIBSICU. My personal preference would be setting STATIC_PREFIX to an empty string, but either approach fixes mh-mingw as far as the static build is concerned.

@srl295
Copy link
Copy Markdown
Member

srl295 commented May 27, 2021

@pchemguy you do need to rebase this on main as mentioned

@pchemguy
Copy link
Copy Markdown
Author

pchemguy commented May 27, 2021

@pchemguy you do need to rebase this on main as mentioned

@srl295
I just did and created #1735

@autoantwort
Copy link
Copy Markdown
Contributor

This PR can be closed in favor of #1735

@pchemguy
Copy link
Copy Markdown
Author

pchemguy commented Jun 8, 2021

Closed in favor of #1735

@pchemguy pchemguy closed this Jun 8, 2021
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.

10 participants