💬 MarcoFalke commented on issue "Intermittent failure in feature_config_args.py":
(https://github.com/bitcoin/bitcoin/issues/28290#issuecomment-1683415295)
I also see this locally on valgrind.

I think the general idea is that we can't measure wall clock time and assume some operation is *faster* than a specified wall clock duration (60 seconds in this case).

Not sure what to do here, other than removing this check?

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298058749)
why does this specify both keys?

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298058494)
Shouldn't the key be just the task name? There should be no reason to bloat the cache directory with entries that won't ever be used again.

See https://github.com/bitcoin/bitcoin/actions/caches

🤔 MarcoFalke reviewed a pull request: "ci: Disable cache save for pull requests in GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/28292#pullrequestreview-1583930912)
concept ACK

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298060542)
Maybe also disable it for branches other than `master`, to make sure that storage is constant, even if there was an intermittent push to a backport branch?

⚠️ mrnadyy opened an issue: " mrmohamednadi@gmail.com"
(https://github.com/bitcoin/bitcoin/issues/28293)

💬 hebasto commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298063957)
While we are interested in the `restore-keys` input only, the `key` input is [required](https://github.com/actions/cache/blob/f7ebb81a3f195b4fb88dab7c14e2f7aff52045aa/restore/action.yml), thus, it cannot be avoided.

💬 MarcoFalke commented on pull request "lint: fix custom mypy cache dir setting":
(https://github.com/bitcoin/bitcoin/pull/28184#discussion_r1298064385)
```suggestion
base_dir = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
```

The two should be the same, no? Seems better to just use one way to get one thing, instead of two.

✅ MarcoFalke closed an issue: " mrmohamednadi@gmail.com"
(https://github.com/bitcoin/bitcoin/issues/28293)

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298066081)
ok, maybe use the same value for both, or is there a reason to use different values?

💬 hebasto commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298067481)
GitHub Actions caches are immutable. Therefore, [this](https://github.com/actions/cache/blob/main/tips-and-workarounds.md#update-a-cache) pattern has been used for Ccache caches.

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298068770)
ugh, fair enough

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298069742)
ok, nvm. Looks like caches are immutable anyway, so this can't be achieved, I guess.

👍 MarcoFalke approved a pull request: "ci: Disable cache save for pull requests in GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/28292#pullrequestreview-1583948672)
lgtm ACK

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298070445)
```suggestion
key: ${{ github.job }}-ccache-${{ github.run_id }} # https://github.com/actions/cache/blob/main/tips-and-workarounds.md#update-a-cache
```

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298072161)
Maybe it is still better to only have caches for a single branch, as they can't be re-used for other branches anyway?

💬 hebasto commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298072950)
> ... is there a reason to use different values?

Yes, it is. `github.run_id` is unique for every run. That is why the `restore-keys` input contains prefixes that are expected to match. The latest matched cache will be restored.

💬 hebasto commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298075170)
In the main repository, caches for backport branches will be evicted after 7 days of non-using them.

However, the current logic seems useful for fork repos, no?

💬 hebasto commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298077810)
The comment has been added.

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298086606)
I was thinking that (assuming a cache size per push of `$n` GB, and a total limit of 10GB), 10/n pushes to a backport branch will evict the main cache. For example, for n=5, 2 pushes are enough.

Though, it is only a cache, so I guess it doesn't matter.

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298093118)
> However, the current logic seems useful for fork repos, no?

If you are worried about hard-coding `master`, I think, if possible, using the GitHub default branch name should achieve the same.