Skip to content

Conversation

fanquake
Copy link
Member

@fanquake fanquake commented Mar 5, 2018

After discussion with @theuni, we can possibly just remove ccache from depends entirely.

Related to #12606

@laanwj
Copy link
Member

laanwj commented Mar 5, 2018

Concept ACK, let's see what Travis says

@practicalswift
Copy link
Contributor

utACK ec7602b39eea0a7f8bc59eca3ac530e96e1e90e6

@maflcko
Copy link
Member

maflcko commented Mar 6, 2018

Looks like configure picked up the ccache: checking if ccache should be used... yes

utACK ec7602b

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Mar 6, 2018

@fanquake fanquake changed the title [WIP] depends: Remove ccache depends: Remove ccache Mar 6, 2018
@maflcko
Copy link
Member

maflcko commented Mar 6, 2018

@jonasschnelli 's gitian build seems fine

@laanwj
Copy link
Member

laanwj commented Mar 6, 2018

What we still need to check is whether travis caches the ccache result, even though ~/.ccache is no longer in the list. I'm not sure how, is it possible to inspect the travis cache?

Edit: I tried through the GUI, as well as the travis command line tool, but with neither I was able to locate the cache for this PR. This might either mean it doesn't exist, or I couldn't get at it for another reason (e.g. some upper limit to the number of shown caches).

Seems the former. At the end of the travis logs it says:

store build cache
nothing changed, not updating cache

It's not writing the cache at all because it detects no changes?!

@laanwj laanwj requested a review from theuni March 6, 2018 14:35
.travis.yml Outdated
directories:
- depends/built
- depends/sdk-sources
- $HOME/.ccache
Copy link
Member

Choose a reason for hiding this comment

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

My guess is that this needs to be re-added

@randolf
Copy link
Contributor

randolf commented Mar 6, 2018

Just in case it's helpful, I successfully complied this on NetBSD 7.0 (64-bit). These are the commands I used:

git clone https://www.github.com/fanquake/bitcoin/
cd bitcoin
git checkout ccache-removal
./autogen.sh
./configure CPPFLAGS="-I/usr/pkg/include" LDFLAGS="-L/usr/pkg/lib" BOOST_CPPFLAGS="-I/usr/pkg/include" BOOST_LDFLAGS="-L/usr/pkg/lib"
gmake

If you'd like me to try building with different parameters, or check for the presence of certain files/directories, please let me know.

@laanwj
Copy link
Member

laanwj commented Mar 6, 2018

@randolf Thanks for testing, however this does not affect normal autotools builds, if you don't use the depends system. And I don't think the depends system works on NetBSD (though never tried).

@randolf
Copy link
Contributor

randolf commented Mar 6, 2018

@laanwj I don't know enough about the "depends system" to provide helpful perspective there, but I'd be happy to try if there are some instructions somewhere that I can follow.

Copy link
Member

@theuni theuni left a comment

Choose a reason for hiding this comment

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

ACK cc87967

@maflcko
Copy link
Member

maflcko commented Mar 6, 2018

re-utACK cc87967

Caching works on arm and osx cross-builds at least

@laanwj
Copy link
Member

laanwj commented Mar 6, 2018

utACK cc87967 - will merge this tomorrow after the meeting to minimize the amount of interference with ongoing travis testing

@maflcko
Copy link
Member

maflcko commented Mar 8, 2018

@laanwj I assume the meeting is over?

@maflcko maflcko merged commit cc87967 into bitcoin:master Mar 8, 2018
maflcko pushed a commit that referenced this pull request Mar 8, 2018
cc87967 depends: Remove ccache (fanquake)

Pull request description:

  After discussion with @theuni, we can possibly just remove ccache from depends entirely.

  Related to #12606

Tree-SHA512: ae0a60c8d97467fa41d617daa48ed22159cf32613808634a983304901dd5ed27124e77868d2314004e5144f7b35ba1333f720bb12daec4c5ca03aaf29d593ef2
@ryanofsky
Copy link
Contributor

The travis builds on all my PR's seem to be failing with errors like:

$ if [ -z "$NO_DEPENDS" ]; then depends/$HOST/native/bin/ccache --max-size=$CCACHE_SIZE; fi
/home/travis/.travis/job_stages: line 57: depends/x86_64-unknown-linux-gnu/native/bin/ccache: No such file or directory

An example is https://travis-ci.org/bitcoin/bitcoin/jobs/350509177#L1055

@maflcko
Copy link
Member

maflcko commented Mar 8, 2018

@ryanofsky This is a travis bug. The command you mention above was removed from the travis yaml, it shouldn't be executed at all.

You can either ignore the failure or rebase your pull request on master. (Just changing the commit id via EDITOR=true git commit --ammend might be enough, but I haven't checked)

Edit: Just checked that rebase is not needed. You can just change the commit id

willyko added a commit to syscoin/syscoin that referenced this pull request Apr 20, 2018
codablock pushed a commit to codablock/dash that referenced this pull request Nov 4, 2019
cc87967 depends: Remove ccache (fanquake)

Pull request description:

  After discussion with @theuni, we can possibly just remove ccache from depends entirely.

  Related to bitcoin#12606

Tree-SHA512: ae0a60c8d97467fa41d617daa48ed22159cf32613808634a983304901dd5ed27124e77868d2314004e5144f7b35ba1333f720bb12daec4c5ca03aaf29d593ef2
@fanquake fanquake deleted the ccache-removal branch December 20, 2019 14:32
barrystyle pushed a commit to PACGlobalOfficial/PAC that referenced this pull request Jan 22, 2020
cc87967 depends: Remove ccache (fanquake)

Pull request description:

  After discussion with @theuni, we can possibly just remove ccache from depends entirely.

  Related to bitcoin#12606

Tree-SHA512: ae0a60c8d97467fa41d617daa48ed22159cf32613808634a983304901dd5ed27124e77868d2314004e5144f7b35ba1333f720bb12daec4c5ca03aaf29d593ef2
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 19, 2020
Summary:
This removes `ccache` from the depends. Host `ccache` will be used
instead if installed.

Backport of core [[bitcoin/bitcoin#12607 | PR12607]].

This will fix a future issue when porting [[bitcoin/bitcoin#12402 | PR12402]], so these PR are
ported in reverse order.

Test Plan: Run the Gitian builds twice and ensure the build is still deterministic.

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5503
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request May 19, 2020
Summary:
This removes `ccache` from the depends. Host `ccache` will be used
instead if installed.

Backport of core [[bitcoin/bitcoin#12607 | PR12607]].

This will fix a future issue when porting [[bitcoin/bitcoin#12402 | PR12402]], so these PR are
ported in reverse order.

Test Plan: Run the Gitian builds twice and ensure the build is still deterministic.

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5503
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants