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

ARROW-2563: [Rust] Poor caching in Travis-CI #2021

Closed
wants to merge 2 commits into from

Conversation

sunchao
Copy link
Member

@sunchao sunchao commented May 10, 2018

No description provided.

@sunchao sunchao force-pushed the arrow-2563 branch 3 times, most recently from ecb1e9a to 4a677ed Compare May 10, 2018 20:18
@sunchao sunchao changed the title [WIP] ARROW-2563: [Rust] Poor caching in Travis-CI ARROW-2563: [Rust] Poor caching in Travis-CI May 10, 2018
@pitrou
Copy link
Member

pitrou commented May 10, 2018

Can you rebase this PR so we can check it works with the codecov upload command?

@sunchao
Copy link
Member Author

sunchao commented May 10, 2018

Sure. Done.

@pitrou
Copy link
Member

pitrou commented May 10, 2018

Thanks. It's down to 10 minutes, which is better, though it seems a lot of third-party stuff still gets recompiled during the build?

@sunchao
Copy link
Member Author

sunchao commented May 11, 2018

Yes. I'm not sure whether there's a way to avoid the recompilation. Will be back.

@pitrou
Copy link
Member

pitrou commented May 11, 2018

Yes, so the problem is that the coverage step panics with that change. I don't know why:
https://travis-ci.org/apache/arrow/jobs/377596737#L731

@sunchao sunchao force-pushed the arrow-2563 branch 2 times, most recently from 3367e93 to 8904cd8 Compare May 12, 2018 07:34
@sunchao
Copy link
Member Author

sunchao commented May 12, 2018

Back. @pitrou : I think the issue is that cargo coverage --verbose tries to download kcov into the target directory, which doesn't exist (because we mapped CARGO_TARGET_DIR to $TRAVIS_BUILD_DIR/target). Therefore, the command failed.

In the latest commit I changed the download location to $CARGO_TARGET_DIR, which fixes this issue. The last run is here and it took 5min 24sec. Can you take another look?

@codecov-io
Copy link

codecov-io commented May 12, 2018

Codecov Report

Merging #2021 into master will increase coverage by 1.14%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2021      +/-   ##
==========================================
+ Coverage   86.28%   87.42%   +1.14%     
==========================================
  Files          11      189     +178     
  Lines         773    29298   +28525     
==========================================
+ Hits          667    25615   +24948     
- Misses        106     3683    +3577
Impacted Files Coverage Δ
cpp/src/plasma/events.h 100% <0%> (ø)
cpp/src/arrow/util/thread-pool.h 92.3% <0%> (ø)
cpp/src/plasma/thirdparty/xxhash.cc 34.98% <0%> (ø)
cpp/src/arrow/python/io.h 100% <0%> (ø)
cpp/src/plasma/events.cc 88.09% <0%> (ø)
cpp/src/arrow/util/memory.h 100% <0%> (ø)
cpp/src/arrow/ipc/reader.cc 87.78% <0%> (ø)
cpp/src/arrow/io/io-memory-test.cc 100% <0%> (ø)
cpp/src/arrow/builder.cc 81.72% <0%> (ø)
cpp/src/arrow/python/decimal.cc 98.93% <0%> (ø)
... and 168 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 321773c...43aa206. Read the comment docs.

@pitrou
Copy link
Member

pitrou commented May 12, 2018

It seems there is a kcov error:
https://travis-ci.org/apache/arrow/jobs/378030262#L1395-L1399

Did it occur before this PR as well?

@sunchao
Copy link
Member Author

sunchao commented May 12, 2018

Hmm... seems still because of the target dir not found.

@sunchao
Copy link
Member Author

sunchao commented May 12, 2018

@pitrou : I changed the code to create a directory target instead, and the latest run succeeded.

@pitrou
Copy link
Member

pitrou commented May 14, 2018

@sunchao, looks great ! Thank you.

@sunchao
Copy link
Member Author

sunchao commented May 14, 2018

Thanks @pitrou . Could you help to merge this?

@pitrou pitrou closed this in 5a29ab8 May 14, 2018
@pitrou
Copy link
Member

pitrou commented May 14, 2018

Done! Thanks for the PR!

@sunchao
Copy link
Member Author

sunchao commented May 14, 2018

Thanks for merging @pitrou !

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.

None yet

3 participants