search

ovalhub / pyicu

The PyICU project repository has moved to https://pyicu.org.
Other
133 stars 49 forks source link

Expand Travis-CI scope #118

Closed SethMMorton closed 4 years ago

SethMMorton commented 4 years ago

This is intended to address and close issue #117.

This PR will take many iterations to get correct, so I have prefixed the title with WIP to indicate it is a work-in-progress. I will remove WIP when I think it is read to merge.

ovalhub commented 4 years ago

On Thu, 14 Nov 2019, Seth Morton wrote:

This is intended to address and close issue #117.

This PR will take many iterations to get correct, so I have prefixed the title with WIP to indicate it is a work-in-progress. I will remove WIP when I think it is read to merge.

  • [ ] Testing against multiple icu4c versions
  • [ ] Testing against multiple Python versions
  • [ ] Testing on Linux
  • [ ] Testing on macos
  • [ ] Testing on Windows
  • [ ] On Linux, testing against both clang and GCC (not sure if this is needed?)

One thing to keep in mind with C++ compilers requirements is that ICU >= 60 (?) requires C++ 11.

You can view, comment on, or merge this pull request online at:

https://github.com/ovalhub/pyicu/pull/118

-- Commit Summary --

  • Travis-CI now recommends to omit sudo altogether
  • Place ICU versions in a matrix
  • Test against all currently support Python versions
  • Move before_install into a script
  • For proof-of-principle, only run tests on 3.7

-- File Changes --

M .travis.yml (42) A install-icu4c-linux.sh (17)

-- Patch Links --

https://github.com/ovalhub/pyicu/pull/118.patch https://github.com/ovalhub/pyicu/pull/118.diff

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/ovalhub/pyicu/pull/118

SethMMorton commented 4 years ago

On macos I get the following message from the setup.py

    FileNotFoundError: [Errno 2] No such file or directory: 'icu-config'

    During handling of the above exception, another exception occurred:

    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/private/var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/pip-req-build-v5paf2q7/setup.py", line 183, in <module>
        raise RuntimeError('''
    RuntimeError:
    Please install pkg-config on your system or set the PYICU_CFLAGS environment
    variable to the flags required by the C++ compiler to find the header files
    for ICU, and possibly -std=c++11 if using ICU version >= 60

    Building PyICU 2.4.2 for ICU 65.1

    (running 'icu-config --cxxflags --cppflags')
    Could not configure CFLAGS with icu-config

How do you build on macos?

SethMMorton commented 4 years ago

Link to above issue: https://travis-ci.com/SethMMorton/pyicu/builds/136610331

ovalhub commented 4 years ago

On Thu, 14 Nov 2019, Seth Morton wrote:

On macos I get the following message from the setup.py

   FileNotFoundError: [Errno 2] No such file or directory: 'icu-config'

   During handling of the above exception, another exception occurred:

   Traceback (most recent call last):
     File "<string>", line 1, in <module>
     File "/private/var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/pip-req-build-v5paf2q7/setup.py", line 183, in <module>
       raise RuntimeError('''
   RuntimeError:
   Please install pkg-config on your system or set the PYICU_CFLAGS environment
   variable to the flags required by the C++ compiler to find the header files
   for ICU, and possibly -std=c++11 if using ICU version >= 60

   Building PyICU 2.4.2 for ICU 65.1

   (running 'icu-config --cxxflags --cppflags')
   Could not configure CFLAGS with icu-config

How do you build on macos?

I build ICU from a source download and it comes with icu-config. cd sources ./runConfigureICU MacOSX make make install

If one doesn't have either icu-config or pkg-config on macos (or elsewhere) installed, they are supposed to edit setup.py or set env vars so that the build gets configured for their environment: edit INCLUDES or get it from PYICU_INCLUDES env var same for CFLAGS or PYICU_CFLAGS LFLAGS or PYICU_LFLAGS LIBRARIES or PYICU_LIBRARIES

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/ovalhub/pyicu/pull/118#issuecomment-554107026

SethMMorton commented 4 years ago

OK. Looks like the .travis-ci.yml previously set up did not use runConfigureICU, but something else. I'll update the build step to "do the right thing", and hopefully that fixes it.

SethMMorton commented 4 years ago

It looks like the jobs have stopped running in Travis... did you disconnect?

ovalhub commented 4 years ago

On Thu, 14 Nov 2019, Seth Morton wrote:

It looks like the jobs have stopped running in Travis... did you disconnect?

I haven't touched anything since setting it up yesterday. Tomorrow, I intend to switch to .org if that proves necessary.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/ovalhub/pyicu/pull/118#issuecomment-554122510

SethMMorton commented 4 years ago

Oh, sorry. I pushed a bad config. It's working now.

SethMMorton commented 4 years ago

Two questions about icu-config

  1. There is no file called that in the ICU repo, so I assume it is created after one of the configure or make step - where is it placed? I'm guessing I need to modify my $PATH to that location to properly build.
  2. In the PyICU setup.py it seems to assume that icu-config is on the $PATH - is this the case in general if icu4c is installed?
ovalhub commented 4 years ago

On Thu, 14 Nov 2019, Seth Morton wrote:

Two questions about icu-config

  1. There is no file called that in the ICU repo, so I assume it is created after one of the configure or make step - where is it placed? I'm guessing I need to modify my $PATH to that location to properly build.
  2. In the PyICU setup.py it seems to assume that icu-config is on the $PATH - is this the case in general if icu4c is installed?

For me, it's installed in /usr/local/bin, so it's on PATH normally. The PyICU user is responsible for a providing a functional ICU installation. That includes having icu-config (which is deprecated, by the way) or pkg-config (the new way, more standard) or setting env vars (or editing setup.py)

In your travis setup, if you're building ICU from scratch, you can configure it so that you know where it's putting its headers and libraries and thus provide env vars instead if you prefer not to depend on icu-|pkg-config.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/ovalhub/pyicu/pull/118#issuecomment-554181639

ovalhub commented 4 years ago

Sorry for the noise, it seems that since 05/2018 all travis-ci setups should be using the .com flavor. https://blog.travis-ci.com/2018-05-02-open-source-projects-on-travis-ci-com-with-github-apps

SethMMorton commented 4 years ago

Sorry for the noise, it seems that since 05/2018 all travis-ci setups should be using the .com flavor.

Got it. Thanks.

SethMMorton commented 4 years ago

@ovalhub OK, I have everything set up except Windows (which I plan to attempt). Here is the link to the build with this setup:

https://travis-ci.com/SethMMorton/pyicu/builds/136782659

This is a LOT of builds. I have set up caching so hopefully ICU does not need to be constantly re-built, just PyICU, but still - Travis-CI has this to say about running a lot of jobs:

If you are on an open-source plan, please remember that Travis CI provides this service free of charge to the community. So please only specify the matrix you actually need.

With that in mind, do you REALLY need to test against all previous ICU versions? Here is my analysis of the failure modes from the linked build:

I think it would be wise to pick a representative selection of active versions, perhaps 65.1, 64.2, 59.2, 57.2, 53.2, 52.2, and 50.2 (although care should be taken to make sure all of 57.2, 56.2, 55.2, 54.2, and 51.3 pass once the undefined symbol issue is fixed).

ovalhub commented 4 years ago

On Fri, 15 Nov 2019, Seth Morton wrote:

@ovalhub OK, I have everything set up except Windows (which I plan to attempt). Here is the link to the build with this setup:

https://travis-ci.com/SethMMorton/pyicu/builds/136782659

  • The currently active Python versions (2.7, 3.5, 3.6, 3.7, and 3.8) are testing against ICU 65.1 and 55.2 (a random non-latest version I chose)

If there are too many builds for 'free' travis-ci plan, dropping Python 3.5 and 3.6 could help.

  • All other ICU versions going back to 50.2 are tested against Python 3.8.

Great !

  • MacOS is tested against ICU 65.1 and Python 3.8 (could do more, but not sure there is much value)

That is the version I develop against, so if I break the build I know immmediately. It'd be more useful to test it against an old ICU, like 55.2. (or whatever version the current macosx is shipping with by default, if that is still the case).

This is a LOT of builds. I have set up caching so hopefully ICU does not need to be constantly re-built, just PyICU, but still - Travis-CI has this to say about running a lot of jobs:

If you are on an open-source plan, please remember that Travis CI provides this service free of charge to the community. So please only specify the matrix you actually need.

Dropping some old Python 3.x versions could help.

With that in mind, do you REALLY need to test against all previous ICU versions? Here is my analysis of the failure modes from the linked build:

No, testing against the version installed on LTS Linux distros would be most useful. For example, this release cycle saw errors against ICU 55.1 (only, so far). Why are people running with such old versions ? probably because that's what their distro gives them ?

  • NameError: name 'LocaleMatcher' is not defined
    • ICU versions 64.2, 63.2, 62.2, 61.2, 60.3, 58.3

LocaleMatcher appeared in ICU 65 and all its mentions should be inside

if ICU > 65 brackets. The fact that you were able to build with ICU 55 (?)

should prove that. Having line numbers for the errors (or a link to the failed build) would be helpful.

  • Compilation errors arising from Edits class
    • ICU version 59.2

The Edits class appeared in ICU 59, and similarly, all its mentions should be bracketed. Same argument as above.

  • Undefined symbol at runtime (something related to FontInstance)
    • ICU versions 57.2, 56.2, 55.2, 54.2, 51.3

That's different. For ICU versions < 58 one must add 'icule' to the libraries to link against (see where this is done in setup.py, line 222). 'icule' contains the defunct LayoutEngine, dropped from ICU >= 58.

  • Missing members from RelativeDateTimeFormatter
    • ICU version 53.2

That one appeared in ICU 53. So it doesn't make sense that it can't be found when testing against ICU 53.2

  • gcc: error: unrecognized command line option '-Qunused-arguments'
    • ICU version 52.2

Are you using an older C++ compiler sometimes ?

  • error: invalid new-expression of abstract class type 'PythonReplaceable' (plus SO MANY compiler warnings)
    • ICU version 50.2

Without the line number, I don't know what to say.

All these errors seems to show that there is something wrong with the setup you're working on. In particular, the build seems to be building with a version of ICU that is different from what the compiler thinks it's seeing. This is all very strange.

I think it would be wise to pick a representative selection of active versions, perhaps 65.1, 64.2, 59.2, 57.2, 53.2, 52.2, and 50.2 (although care should be taken to make sure all of 57.2, 56.2, 55.2, 54.2, and 51.3 pass once the undefined symbol issue is fixed).

Agreed: the ICU versions to test against should be the ones included in Linux LTS distros (first). In recent Linux distros (next). On Windows (a recent and an old ICU version). On MacOS, an old ICU version, ideally pre C++ 11, say ICU 55.

Thank you very much for all the work so far !

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/ovalhub/pyicu/pull/118#issuecomment-554584096

SethMMorton commented 4 years ago

As far as finding the line numbers of errors - all of that information can be found in the Travis-CI link I put in my last comment.

Also, I am not actively changing c++ versions - just using what the setup.py finds.

From my perspective the fact that the CI is running and finding errors is a success. My plan was to get the CI up-and-running (even if there are build errors unrelated to the CI) and then hand it over. Is this what you had in mind?

SethMMorton commented 4 years ago

The fact that you were able to build with ICU 55 (?) should prove that.

ICU 55 build failed.

ovalhub commented 4 years ago

Weren't you able, outside of travis, to build with icu 55 ? (isn't this how this whole thread got started)

On Nov 15, 2019, at 17:34, Seth Morton notifications@github.com wrote:

The fact that you were able to build with ICU 55 (?) should prove that.

ICU 55 build failed.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

ovalhub commented 4 years ago

On Nov 15, 2019, at 17:34, Seth Morton notifications@github.com wrote:

As far as finding the line numbers of errors - all of that information can be found in the Travis-CI link I put in my last comment.

Also, I am not actively changing c++ versions - just using what the setup.py finds.

From my perspective the fact that the CI is running and finding errors is a success. My plan was to get the CI up-and-running (even if there are build errors unrelated to the CI) and then hand it over. Is this what you had in mind?

If there are build errors related to the CI, it's less than ideal but I can proceed - your work is valuable nonetheless...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

ovalhub commented 4 years ago

The ICU 55.2 build failed because of this error: undefined symbol: _ZTIN6icu_5514LEFontInstanceE This means that LayoutEngine wasn't linked in. The CI setup must include 'icule' in the libraries for ICU versions < 58 (as is done in setup.py, line 223)

SethMMorton commented 4 years ago

If there are build errors related to the CI, it's less than ideal but I can proceed

Just to be clear, I meant errors unrelated to the CI (e.g. the CPP code needs fixing). I do not plan to hand you a broken CI configuration.

Weren't you able, outside of travis, to build with icu 55

I used the ICU package Ubuntu within Travis-CI, which apparently is version 55. It's pre-built. I suppose since it is distro-installed it sets up all the environment variables correctly so that PyICU finds all that it needs. In the CI I have to set up the environment manually, so it's easy to miss stuff.

This means that LayoutEngine wasn't linked in. The CI setup must include 'icule' in the libraries for ICU versions < 58 (as is done in setup.py, line 223)

I see. If you could help me by spelling out other fixes I need to make like this one that would be helpful. I am not as familiar as what is needed to get everything working as you.

ovalhub commented 4 years ago

I installed ICU 55.2 and the build builds but the tests fail since the conditional code added to if name == "main" doesn't seem to have an effect when running from setup.py test. Working on a fix...

ovalhub commented 4 years ago

PyICU on trunk, buildt with ICU 55.2 sources now passes all tests with Python 3.8, 2.7.16 on MacOS.

ovalhub commented 4 years ago

PyICU on trunk (unchaged), built with ICU 58.3 sources now passes all tests with Python 3.8, 2.7.16 on MacOS.

ovalhub commented 4 years ago

PyICU on trunk, built with ICU 59.2 sources now passes all tests with Python 3.8, 2.7.16 on MacOS.

ovalhub commented 4 years ago

PyICU on trunk, built with ICU 50.2 sources now passes all tests with Python 3.8, 2.7.16 on MacOS.

ovalhub commented 4 years ago

PyICU on trunk (unchanged), built with ICU 61.2 sources now passes all tests with Python 3.8, 2.7.16 on MacOS.

ovalhub commented 4 years ago

PyICU on trunk (unchanged), built with ICU 57.2 sources now passes all tests with Python 3.8, 2.7.16 on MacOS.

ovalhub commented 4 years ago

PyICU on trunk (unchanged), built with ICU 63.2 sources now passes all tests with Python 3.8, 2.7.16 on MacOS.

ovalhub commented 4 years ago

I've now tested a bunch of older ICU versions locally, on Mac OS, and fixed the issues found. I'm stopping now, waiting to see if CI finds more issues to be resolved. (you still need to add icule to PYICU_LIBRARIES when ICU < 58, for the older builds to succeed)

SethMMorton commented 4 years ago

Wow! That fixed nearly all the issues!

There are still two left:

ovalhub commented 4 years ago

On Sat, 16 Nov 2019, Seth Morton wrote:

Wow! That fixed nearly all the issues!

There are still two left:

I just committed a fix for that one now.

  • ICU 52.2 (https://travis-ci.com/SethMMorton/pyicu/jobs/257267071)
    • I'm completely at a loss here. Whatever mechanism is deciding the CFLAGs to add to the gcc call is adding -Qunused-arguments for this ICU version and this ICU version only. The interesting thing is that -Qunused-arguments has never been a gcc option to my knowledge, but it is a clang option. I am observing (in general) that ICU is being built with clang but PyICU is being built with gcc (typically OK, they are binary compatible). My guess is that for whatever reason the command-line arguments are getting mixed up for this ICU version only, but that's just a guess.

I can't reproduce this here. Some of the compiler flags are set by python's setuptools itself and depend how python was compiled. If that version, 52.2, is the only one with this problem, it's certainly ok to drop it from the CI. We have plenty of versions already...

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/ovalhub/pyicu/pull/118#issuecomment-554664735

ovalhub commented 4 years ago

On Sat, 16 Nov 2019, Seth Morton wrote:

Wow! That fixed nearly all the issues!

There are still two left:

  • ICU 53.2 (https://travis-ci.com/SethMMorton/pyicu/jobs/257267070)

    • There are still missing member name compilation errors from the RelativeDateTimeFormatter class

  • ICU 52.2 (https://travis-ci.com/SethMMorton/pyicu/jobs/257267071)

    • I'm completely at a loss here. Whatever mechanism is deciding the CFLAGs to add to the gcc call is adding -Qunused-arguments for this ICU version and this ICU version only. The interesting thing is that -Qunused-arguments has never been a gcc option to my knowledge, but it is a clang option. I am observing (in general) that ICU is being built with clang but PyICU is being built with gcc (typically OK, they are binary compatible). My guess is that for whatever reason the command-line arguments are getting mixed up for this ICU version only, but that's just a guess.

That one seems to come from icu-config, see log:

(running 'icu-config --cxxflags --cppflags') 2304 Adding CFLAGS="-O3 -W -Wall -pedantic -Wpointer-arith -Wwrite-strings -Wno-long-long --std=c++0x -Qunused-arguments -Wno-parentheses-equality -D_REENTRANT -DU_HAVE_ELF_H=1 -DU_HAVE_ATOMIC=1 -I/home/travis/icu4c/include" from /home/travis/icu4c/bin/icu-config

Also, the --std=c++0x in there is suspicious. C++11 became a requirement much later than ICU 52.2. Are you sure that you're running the icu-config that corresponds to version of ICU you're testing against ?

SethMMorton commented 4 years ago

Are you sure that you're running the icu-config that corresponds to version of ICU you're testing against?

Not 100%. But I don't see how it could be otherwise - ICU isn't installed on the system and I'm only installing the one version per container. I'll update the configuration to echo a bunch of configuration information so that we can be more sure.

I can't reproduce this here.

My guess is because you are on macos and clang is the default compiler there. I wonder if you could reproduce if you set CC to gcc before executing setup.py.

ovalhub commented 4 years ago

On Sat, 16 Nov 2019, Seth Morton wrote:

Wow! That fixed nearly all the issues!

There are still two left:

  • ICU 53.2 (https://travis-ci.com/SethMMorton/pyicu/jobs/257267070)
    • There are still missing member name compilation errors from the RelativeDateTimeFormatter class
  • ICU 52.2 (https://travis-ci.com/SethMMorton/pyicu/jobs/257267071)
    • I'm completely at a loss here. Whatever mechanism is deciding the CFLAGs to add to the gcc call is adding -Qunused-arguments for this ICU version and this ICU version only. The interesting thing is that -Qunused-arguments has never been a gcc option to my knowledge, but it is a clang option. I am observing (in general) that ICU is being built with clang but PyICU is being built with gcc (typically OK, they are binary compatible). My guess is that for whatever reason the command-line arguments are getting mixed up for this ICU version only, but that's just a guess.

Also, to be more explicit you're testing against the intended version of ICU, it'd be nice to have the version of ICU embedded into all the paths used: like you have -I/opt/python/3.8.0/include/python3.8 you should also have -I/home/travis/icu-52.2/include instead of -I/home/travis/icu4c/include

Same for $PATH, $LD_LIBRARY_PATH, etc...

On my mac, to test all these versions locally, without polluting my env, here is what I did to build and install ICU into its own tree:

ovalhub commented 4 years ago

On Sat, 16 Nov 2019, Seth Morton wrote:

Are you sure that you're running the icu-config that corresponds to version of ICU you're testing against?

Not 100%. But I don't see how it could be otherwise - ICU isn't installed on the system and I'm only installing the one version per container. I'll update the configuration to echo a bunch of configuration information so that we can be more sure.

I can't reproduce this here.

My guess is because you are on macos and clang is the default compiler there. I wonder if you could reproduce if you set CC to gcc before executing setup.py.

On my mac, gcc and clang are identical, both say Apple LLVM version 8.1.0 (clang-802.0.42) Target: x86_64-apple-darwin18.6.0 Thread model: posix InstalledDir: /Library/Developer/CommandLineTools/usr/bin

SethMMorton commented 4 years ago

On my mac, gcc and clang are identical

Right, I forgot about that.

SethMMorton commented 4 years ago

It'd be nice to have the version of ICU embedded into all the paths used

I had originally done that, but found it to make the caching process more challenging and it wasn't necessary. I can bring it back if you'd like, but I think that the solution I just came up with should be fine. Keep in mind that Travis-CI uses a container (Docker) based mechanism, so each job is a "clean slate" so there is no mixing between them. It is really not possible for the data from one job to "leak" into another, so it's not possible to accidentally compile against the wrong ICU version unless one was already installed (which is not the case) or I downloaded the wrong one (unlikely).

ovalhub commented 4 years ago

On Sat, 16 Nov 2019, Seth Morton wrote:

It'd be nice to have the version of ICU embedded into all the paths used

I had originally done that, but found it to make the caching process more challenging and it wasn't necessary. I can bring it back if you'd like, but I think that the solution I just came up with should be fine. Keep in mind that Travis-CI uses a container (Docker) based mechanism, so each job is a "clean slate" so there is no mixing between them. It is really not possible for the data from one job to "leak" into another, so it's not possible to accidentally compile against the wrong ICU version unless one was already installed (which is not the case) or I downloaded the wrong one (unlikely).

That makes sense, I didn't realize that. What does the cache work, then, if it's all hermetic, in containers ?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/ovalhub/pyicu/pull/118#issuecomment-554668982

SethMMorton commented 4 years ago

All of Travis's caching documentation are here: https://docs.travis-ci.com/user/caching/. I'm not quite sure what you are asking, but think this will answer your question: https://docs.travis-ci.com/user/caching/#how-does-caching-work.

SethMMorton commented 4 years ago

This may also be of interest: https://docs.travis-ci.com/user/caching/#caches-and-build-matrices

SethMMorton commented 4 years ago

It seems like the --cxxflags output for ICU version 52.2 is broken. I am going to skip this version in the CI.

SethMMorton commented 4 years ago

All green!

With the new configuration information being output, I am now 100% confident the correct ICU is being used in all cases. I also note something interesting...

Also, the --std=c++0x in there is suspicious. C++11 became a requirement much later than ICU 52.2.

For ICU version 50.2 and 51.3, they were including the -std=c++11 flag, and then starting in 52.2 switched to -std=c++0x, then switched back at a later point.

ovalhub commented 4 years ago

Excellent ! Is your PR still WIP or can I merge it ?

On Nov 16, 2019, at 13:27, Seth Morton notifications@github.com wrote:

All green!

With the new configuration information being output, I am now 100% confident the correct ICU is being used in all cases. I also note something interesting...

Also, the --std=c++0x in there is suspicious. C++11 became a requirement much later than ICU 52.2.

For ICU version 50.2 and 51.3, they were including the -std=c++11 flag, and then starting in 52.2 switched to -std=c++0x, then switched back at a later point.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

SethMMorton commented 4 years ago

Still WIP. I plan to add windows support. I'm excited to do that because I will apply what I learn here to my repos.

ovalhub commented 4 years ago

On Nov 16, 2019, at 13:50, Seth Morton notifications@github.com wrote:

Still WIP. I plan to add windows support. I'm excited to do that because I will apply what I learn here to my repos.

Cool, thank you !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

SethMMorton commented 4 years ago

All right... I'm about ready to give up getting Windows to work. After many failed attempts to even get it start compiling, it was able to compile for 13 minutes and then fail.

https://travis-ci.com/SethMMorton/pyicu/jobs/257412314

Looks like they invoke some python script near the end of the build, and it crashes because it assumes some item was in a list but it is not.

Anyway... how much do you want Windows support in CI?

ovalhub commented 4 years ago

On Sat, 16 Nov 2019, Seth Morton wrote:

All right... I'm about ready to give up getting Windows to work. After many failed attempts to even get it start compiling, it was able to compile for 13 minutes and then fail.

From the logs, it looks like you're building a bunch of different windows configurations. Reading the ICU readme.html about installing on Windows, they seem to indeed have a bunch of variants: cygwin windows, then 32bit, 64bit, UWP (what is that ?), Release, Debug, ... I think you should pick just one, say 64-bit, non UWP, Release, build so that the MSVC compiler is used (it has been the source of issues in the past). Then, follow the ICU instructions to build using the command line using msbuild or devenv.com. I saw that Travis considers "Windows builds an early access feature"...

https://travis-ci.com/SethMMorton/pyicu/jobs/257412314

Looks like they invoke some python script near the end of the build, and it crashes because it assumes some item was in a list but it is not.

Anyway... how much do you want Windows support in CI?

It would be useful to have since I have no access to Windows at all. But it's ok to give up if this proves too much of a pain (!)

SethMMorton commented 4 years ago

From the logs, it looks like you're building a bunch of different windows configurations.

I don't think you are reading the logs correctly. I am only trying to get ICU built with MSVC. I attempted to do this with either cygwin (as in this build: https://travis-ci.com/SethMMorton/pyicu/jobs/257411610, which fails because it thinks the C compiler doesn't work) or the VS studio suite (as in this build: https://travis-ci.com/SethMMorton/pyicu/builds/136856342 which fails for who knows why) and neither worked. Each of these were in separate builds, e.g. different commits. At no point did I try to do both at the same time, that would be stupid.

I think you should pick just one, say 64-bit, non UWP, Release, build so that the MSVC compiler is used (it has been the source of issues in the past). Then, follow the ICU instructions to build using the command line using msbuild or devenv.com.

That is exactly what I did in the job I linked to... have you taken a look at the code I am commiting? If not, here's a link to the line where I use the msbuild command (which I did read from the ICU README).

https://github.com/ovalhub/pyicu/blob/f394b35cecb3012f1effb5e29b81a775eaedb5db/ci-prepare-environment.bash#L114

And here's the line where that failed due to an error I have no idea how to debug: https://travis-ci.com/SethMMorton/pyicu/jobs/257412314#L3593.

ovalhub commented 4 years ago

On Nov 17, 2019, at 10:50, Seth Morton notifications@github.com wrote:

From the logs, it looks like you're building a bunch of different windows configurations.

I don't think you are reading the logs correctly. I am only trying to get ICU built with MSVC. I attempted to do this with either cygwin (as in this build: https://travis-ci.com/SethMMorton/pyicu/jobs/257411610, which fails because it thinks the C compiler doesn't work) or the VS studio suite (as in this build: https://travis-ci.com/SethMMorton/pyicu/builds/136856342 which fails for who knows why) and neither worked. Each of these were in separate builds, e.g. different commits. At no point did I try to do both at the same time, that would be stupid.

I think you should pick just one, say 64-bit, non UWP, Release, build so that the MSVC compiler is used (it has been the source of issues in the past). Then, follow the ICU instructions to build using the command line using msbuild or devenv.com.

That is exactly what I did in the job I linked to...

Last night, I followed the link you sent me and, finding the build logs, I saw the list of ICU files fly several times. I did not look at your code then. I also saw that the DLLs seemed to have been built and saw the failure you reported with py.exe (which I'm just as mystified about as you). have you taken a look at the code I am commiting? If not, here's a link to the line where I use the msbuild command (which I did read from the ICU README).

https://github.com/ovalhub/pyicu/blob/f394b35cecb3012f1effb5e29b81a775eaedb5db/ci-prepare-environment.bash#L114

And here's the line where that failed due to an error I have no idea how to debug: https://travis-ci.com/SethMMorton/pyicu/jobs/257412314#L3593.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

ovalhub commented 4 years ago

On Nov 17, 2019, at 10:50, Seth Morton notifications@github.com wrote:

From the logs, it looks like you're building a bunch of different windows configurations.

I don't think you are reading the logs correctly. I am only trying to get ICU built with MSVC. I attempted to do this with either cygwin (as in this build: https://travis-ci.com/SethMMorton/pyicu/jobs/257411610, which fails because it thinks the C compiler doesn't work) or the VS studio suite (as in this build: https://travis-ci.com/SethMMorton/pyicu/builds/136856342 which fails for who knows why) and neither worked. Each of these were in separate builds, e.g. different commits. At no point did I try to do both at the same time, that would be stupid.

I think you should pick just one, say 64-bit, non UWP, Release, build so that the MSVC compiler is used (it has been the source of issues in the past). Then, follow the ICU instructions to build using the command line using msbuild or devenv.com.

That is exactly what I did in the job I linked to... have you taken a look at the code I am commiting? If not, here's a link to the line where I use the msbuild command (which I did read from the ICU README).

https://github.com/ovalhub/pyicu/blob/f394b35cecb3012f1effb5e29b81a775eaedb5db/ci-prepare-environment.bash#L114

I remember seeing something last night in the icu readme about adding something to the command line about skipping uwp (not near laptop right now to be more precise)

And here's the line where that failed due to an error I have no idea how to debug: https://travis-ci.com/SethMMorton/pyicu/jobs/257412314#L3593.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.