Skip to content

doc: fix the logging variable that disables client-side mode - #2969

Merged
sylvestre merged 1 commit into
mozilla:mainfrom
tiny-ac:fix/client-side-logging-docs
Oct 29, 2026
Merged

sylvestre merged 1 commit into
mozilla:mainfrom
tiny-ac:fix/client-side-logging-docs

Conversation

@tiny-ac

@tiny-ac tiny-ac commented Oct 29, 2026

Copy link
Copy Markdown

Description

Configuration.md and Architecture.md say client-side mode is disabled when the SCCACHE_ERROR_LOG environment variable is set, but the code actually checks the SCCACHE_LOG env variable.

SCCACHE_ERROR_LOG is read when the daemon starts to redirect stderr and it's also mentioned in the code comment where the client-side check runs, but the code actually reads LOGGING_ENV, a constant that holds the environment variable name (SCCACHE_LOG).

Ran a compile with a fresh cache and a freshly started server:

  • With SCCACHE_CLIENT_SIDE=1, setting only SCCACHE_ERROR_LOG keeps client-side mode
    • sccache --show-stats shows Compile requests 0
  • SCCACHE_LOG=warn switches back to server-side mode
    • sccache --show-stats shows Compile requests 1

Changes

This PR changes both docs to name SCCACHE_LOG instead of SCCACHE_ERROR_LOG.

Comment thread docs/Architecture.md Outdated
removed. It is currently mutually exclusive with:

- **error logging** (`SCCACHE_ERROR_LOG`): the client always logs to stderr, and
- **logging** (`SCCACHE_LOG`): the client always logs to stderr, and

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe make the reason match the new variable? something like:

Suggested change
- **logging** (`SCCACHE_LOG`): the client always logs to stderr, and
- **logging** (`SCCACHE_LOG`): the client always logs to stderr, ignoring
`SCCACHE_ERROR_LOG`, and multiple concurrent CLI processes would race on the log file, and

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@sylvestre Thanks! Applied with a small rewording to avoid the double "and"

@codecov-commenter

codecov-commenter commented Oct 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.54%. Comparing base (dac6c80) to head (fb2fed6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2969      +/-   ##
==========================================
- Coverage   78.54%   78.54%   -0.01%     
==========================================
  Files          74       74              
  Lines       44039    44039              
==========================================
- Hits        34591    34590       -1     
- Misses       9448     9449       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tiny-ac
tiny-ac force-pushed the fix/client-side-logging-docs branch from 06171ae to fb2fed6 Compare October 29, 2026 16:17
@tiny-ac
tiny-ac requested a review from sylvestre October 29, 2026 16:20
@sylvestre
sylvestre merged commit b937482 into mozilla:main Oct 29, 2026
51 checks passed
@sylvestre

Copy link
Copy Markdown
Collaborator

Thanks for your PR

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.

3 participants