Skip to content

Add travis build: the third try #5614

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

Merged
merged 15 commits into from
Oct 11, 2017
Merged

Conversation

saschanaz
Copy link
Collaborator

@saschanaz saschanaz commented Sep 30, 2017

Basically copied from #5087. Probably the test should be more than just a hello world.

PS: Please merge emscripten-core/emsdk#106 to reduce the build log.

@saschanaz saschanaz force-pushed the travis branch 7 times, most recently from 75a89e5 to 20a3df5 Compare September 30, 2017 14:23
@saschanaz
Copy link
Collaborator Author

saschanaz commented Sep 30, 2017

Any clue why test_time and SSE tests fail? They also fail on my local Ubuntu installation.

BTW, it seems the core test set is too bulky for Travis.

@saschanaz
Copy link
Collaborator Author

I added -v and the result is:

clang -cc1 version 4.0.0 based upon LLVM 4.0.0 default target x86_64-unknown-linux-gnu
ignoring nonexistent directory "/home/saschanaz/emsdk-portable/clang/e1.37.21_64bit/../lib/clang/4.0.0/include"
ignoring nonexistent directory "/include"
ignoring duplicate directory "/usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/x86_64-linux-gnu/c++/5.4.0"
#include "..." search starts here:
#include <...> search starts here:
 /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0
 /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/x86_64-linux-gnu/c++/5.4.0
 /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/backward
 /usr/local/include
 /usr/include/x86_64-linux-gnu
 /usr/include
End of search list.
/home/saschanaz/emsdk-portable/emscripten/1.37.21/tests/test_sse1_full.cpp:4:10: fatal error: 'xmmintrin.h' file not
      found
#include <xmmintrin.h>
         ^~~~~~~~~~~~~
1 error generated.
ERROR

...while the required header is here: /usr/lib/gcc/x86_64-linux-gnu/5/include

@saschanaz
Copy link
Collaborator Author

Filed #5617 and #5618.

@kripken
Copy link
Member

kripken commented Oct 2, 2017

Nice, thanks for looking into this! This works better than I expected already.

Some issues to think about here:

  1. The travis timeout won't let us run all the test suite, it's just too big. One option might be to pick a subset ourselves that is small enough. Or we could do random100 or some other number, which does a bunch of random tests - upside is coverage over time would be good, downside is less consistency.
  2. This downloads the emsdk and uses it, but better would be for it to use the emscripten we are in - so the PR's changes are testsed - with the emsdk's binary LLVM+clang. I think the easiest way to do that is (a) probably to not activate the emsdk, and (b) set up the ~/.emscripten file ourselves manually, or to set the env var LLVM to point to the emsdk's LLVM/

As for those two failing tests, I think it's ok to ignore them on the bots. We'd likely have to make a list of them, as I think @juj has on his bots.

@saschanaz
Copy link
Collaborator Author

  1. Tried a funny parallelization 😄 by names.
  2. Added a new PATH=/:$PATH line after activation, this will force CI to use the PR's code.
  3. Skipped those failing tests.

@juj
Copy link
Collaborator

juj commented Oct 3, 2017

How long is the maximum time that Travis allows to run? And how long would the current test suites in full take up when run on Travis level hardware?

I see that there is a new project that has come up to enable buildbot to integrate with GitHub, done by Rust people: https://github.com/graydon/bors. It might be possible to use something like that to integrate these "build PRs before merging" checks with existing buildbot configs, in which case it would be possible to add the existing buildbot arch to run through PRs, without time limitations.

@saschanaz
Copy link
Collaborator Author

saschanaz commented Oct 4, 2017

How long is the maximum time that Travis allows to run?

40 50 minutes per job.

And how long would the current test suites in full take up when run on Travis level hardware?

Not sure, but it can only cover test_t* to test_z* within 40 minutes. (It takes hours on my local machine, I couldn't even wait for it to finish)

@kripken
Copy link
Member

kripken commented Oct 4, 2017

Heh, nice parallelization ;) Sometimes the simplest solutions work surprisingly well.

There's more tests that we would want to run. In particular, ALL.test_a* instead of test_a* would run those tests in all optimization modes. Probably 5x slower, though... Also, the other tests would be good, but they probably take too long by themselves.

Overall, I think all the tests we'd like to run are going to be too much to Travis, even split up into chunks - I'm sure they'll limit our resources eventually. So we'd need to decide what subset we want to run. Or, figure out what the travis limits are - they do have paid plans, and maybe we can get that set up, but I'm not sure from the page there what the limits are for open source, maybe someone here knows?

@saschanaz
Copy link
Collaborator Author

saschanaz commented Oct 5, 2017

Added ALL.test_ and other, while I had to split test_d* and test_p* into smaller pieces.

but I'm not sure from the page there what the limits are for open source

It seems it's not well documented, but 50-min build timeout and five concurrent jobs.

Travis said in 2015 that further support for an open source project can be discussed via email.

@saschanaz saschanaz closed this Oct 6, 2017
@saschanaz saschanaz reopened this Oct 6, 2017
`test-binaryen` started to fail on the latest prebuilt SDK 1.37.21 after emscripten-core#5620 as the SDK doesn't support `--trap-mode` yet. Disabling the test.
@saschanaz
Copy link
Collaborator Author

test-binaryen started to fail on the latest prebuilt SDK 1.37.21 after #5620 as the SDK doesn't support --trap-mode yet. Disabling the test.

to make abspath test pass (which expects warnings which will not appear on system root directory)
test_native_link_error_message is skipped because of clang library issue (system doesn't have stddef.h depended by stdio.h, only gcc does)

others are skipped because of missing --trap-mode issue.
It also has clang library problem.
@curiousdannii
Copy link
Contributor

curiousdannii commented Oct 7, 2017

So it's taking 9 hours overall!

Looks like it takes about 4 minutes to set up Emscripten for each build job (as well as some time to wait for a VM to be ready). For any of the build jobs which run less than 15 minutes or so it would probably be faster overall to combine them together again.

@saschanaz
Copy link
Collaborator Author

saschanaz commented Oct 9, 2017

Currently a single test fails because the version tag of the incoming branch and the latest precompiled SDK don't match. Does this happen frequently, should I worry about it?

@kripken
Copy link
Member

kripken commented Oct 10, 2017

Travis said in 2015 that further support for an open source project can be discussed via email.

Thanks @saschanaz for finding those links! It seems like we're ok then to run all these jobs, they just won't be concurrent, so it'll take a while for them all. But I think that's ok for us.

Currently a single test fails because the version tag of the incoming branch and the latest precompiled SDK don't match. Does this happen frequently, should I worry about it?

I'm not sure how quickly the precompiled SDK updates to the latest version - it should be as fast as the bots can build it, I think, @juj?

Aside from understanding that issue, I think this PR is good to merge. Nice work!

@saschanaz
Copy link
Collaborator Author

saschanaz commented Oct 11, 2017

It seems like we're ok then to run all these jobs, they just won't be concurrent, so it'll take a while for them all.

They run concurrently, so that the actual full running time can be 1 hr 33 min while the total sum of the running times of each job is 6 hrs 41 min.

@saschanaz
Copy link
Collaborator Author

saschanaz commented Oct 11, 2017

No 1.37.22 tag on binaryen side, probably that's why no precompiled SDK 1.37.22?

@kripken
Copy link
Member

kripken commented Oct 11, 2017

Good point, I keep forgetting that. I tagged binaryen at 1.37.22 now. Should probably automate that somehow.

@juj
Copy link
Collaborator

juj commented Oct 11, 2017

How long is the maximum time that Travis allows to run?
40 50 minutes per job.

And how long would the current test suites in full take up when run on Travis level hardware?
Not sure, but it can only cover test_t* to test_z* within 40 minutes. (It takes hours on my local machine, I couldn't even wait for it to finish)

For reference, the current physical build bots take up

  • ~8h on Ubuntu (4-core Mac Mini from 2011),
  • 2h 50 min on OS X (12-core Mac Pro workstation),
  • 4 hours on Windows (8-core Windows 10 Pro from 2017)

I'm not sure how quickly the precompiled SDK updates to the latest version - it should be as fast as the bots can build it, I think, @juj?

The bots do full builds from scratch (not incremental builds) when they see new tags on all four repositories. These builds generally take up a couple of hours, but they run serial with respect to the unit test jobs, so it is possible that e.g. a 8h unit test run on Ubuntu gets queued before doing a tag rebuild, in which case it may take something like 8+2 = 10 hours to see the new tagged build come up.

@kripken
Copy link
Member

kripken commented Oct 11, 2017

Ok, I think we can merge this in. That other.test_emcc error should vanish eventually, and everything else looks green. And landing this now can help with a few large PRs that are almost ready to land.

In the long term, we may want to add more tests using @juj's physical bots so that we also get the browser tests, etc., so we can refine and improve this over time, but this PR is a great step. Thanks a lot @saschanaz!

@kripken kripken merged commit cf50894 into emscripten-core:incoming Oct 11, 2017
@saschanaz saschanaz deleted the travis branch October 12, 2017 00:13
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.

4 participants