Page MenuHomeFreeBSD

bsd.gecko.mk: add conditional for ccache
Needs ReviewPublic

Authored by vishwin on Feb 1 2018, 4:27 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 20, 12:31 AM
Unknown Object (File)
Oct 7 2024, 5:27 PM
Unknown Object (File)
Oct 5 2024, 4:47 AM
Unknown Object (File)
Oct 1 2024, 9:21 AM
Unknown Object (File)
Sep 30 2024, 5:23 AM
Unknown Object (File)
Sep 24 2024, 9:58 AM
Unknown Object (File)
Sep 24 2024, 4:28 AM
Unknown Object (File)
Sep 12 2024, 4:35 PM

Details

Reviewers
jbeich
salvadore
Group Reviewers
gecko
Summary

See PR 224471. I've been using this since at least a week before I filed that PR with great success.

Test Plan
  • Apply patch, set WITH_CCACHE_BUILD in make.conf
  • Build gecko ports as usual, then build again to utilise the cache

Diff Detail

Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 21175
Build 20530: arc lint + arc unit

Event Timeline

Fix conditional, especially when running make config.

Workaround for static ccache in poudriere. /ccache hierarchy isn't in $PATH.

salvadore added a subscriber: salvadore.

I tested successfully the patch for www/firefox.

This revision is now accepted and ready to land.Sep 5 2019, 1:59 PM

This revision is accepted and ready to land since months. Can a committer commit it please?

Thanks!

Looks OK in generaly but see inline comments. I've never used ccache with ports and unlikely to do so in future.

Mk/bsd.gecko.mk
150
  • Can you move the whole conditional close to where .if ${PORT_OPTIONS:M...} conditonals are?
  • Don't you need && !defined(NO_CCACHE) like other ports with defined(WITH_CCACHE_BUILD) ?
152

Isn't BUILD_DEPENDS already handled by Mk/bsd.ccache.mk?

Mk/bsd.gecko.mk
152

Unfortunately not in this (edge) case. Especially in poudriere, if using the host's statically-linked ccache, the binary gets copied to the reference jail's /ccache/bin/ccache, which is not in $PATH. The mozilla build system only looks for ccache in $PATH, so without this line, the build fails when using host-based ccache.

what needs to happen to get this tested, approved, and moving?

Is this still accepted as it was in 2019?

If not – if a change is inarguably required: please, can someone suggest a change to the code?

Many thanks

After about 4 years since my tests, I have no idea if this patch still works or not. It should be tested again, but unfortunately I do not have the time to do it. I hope someone else will be able to do it.

This revision now requires review to proceed.Apr 23 2023, 12:42 PM