Conversation
…Tools-only machines Cargo's [env] table is static TOML with no way to check whether a path exists, so the unconditional DEVELOPER_DIR override pointed every cargo invocation -- not just the GUI's Metal shader compile it was meant to help -- at /Applications/Xcode.app the moment that path didn't exist: Command Line Tools-only installs, or Xcode under a non-standard name like Xcode-beta.app. Every dependency's build script then failed to link, so cargo build/check/test never got past compiling the workspace's own crates. force = false never prevented this: it only yields to a DEVELOPER_DIR already exported in the calling shell, which a plain xcode-select misconfiguration never sets on its own. devenv (devenv.nix) and CI (.github/workflows/*.yml) already set their own correct DEVELOPER_DIR independently of this file, so removing it doesn't affect either. A contributor building the GUI outside devenv with real Xcode installed but xcode-select pointed at the Command Line Tools keeps the one-time system fix documented in the removed block's replacement comment. Fixes AprilNEA#1079
|
Comment on lines
+32
to
+34
| # sets. devenv (`devenv.nix`) and CI (`.github/workflows/*.yml`) already set | ||
| # their own correct `DEVELOPER_DIR` independently of this file, so removing | ||
| # it doesn't affect either. A contributor building the GUI outside devenv |
There was a problem hiding this comment.
The comment says CI workflows set their own DEVELOPER_DIR, but the macOS checks and tests in .github/workflows/ci.yml do not set it and instead rely on the hosted runner’s selected Xcode. Narrow this statement to the build and release workflows so it does not mislead future build debugging.
Suggested change
| # sets. devenv (`devenv.nix`) and CI (`.github/workflows/*.yml`) already set | |
| # their own correct `DEVELOPER_DIR` independently of this file, so removing | |
| # it doesn't affect either. A contributor building the GUI outside devenv | |
| # sets. devenv (`devenv.nix`) and the build/release workflows set their own | |
| # correct `DEVELOPER_DIR` independently of this file, so removing it doesn't | |
| # affect those paths. A contributor building the GUI outside devenv |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.cargo/config.tomlunconditionally setDEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer(withforce = false) for every macOS cargo invocation, meant as a "belt-and-suspenders" fallback for a contributor who has Xcode installed butxcode-selectpointed at the Command Line Tools.[env]table is static TOML — there is no way to check whether that path actually exists before applying it. On a machine with only the Command Line Tools installed (no Xcode.app at all), or Xcode installed under a non-standard name likeXcode-beta.app, this pointed every dependency's build-script linker step at a path that doesn't exist.cargo build/check/testfailed before compiling a single workspace crate — not just the GUI's Metal shader step this was meant to help.force = falsenever protected against this: it only yields to aDEVELOPER_DIRalready exported in the calling shell, which a plainxcode-selectmisconfiguration never sets on its own — so the override always won in exactly the broken case.Changes
crates/.cargo/config.toml: removed the static[env] DEVELOPER_DIRblock, replaced with a comment explaining why and pointing at the one-time system fix (sudo xcode-select -s /Applications/Xcode.app/Contents/Developer) for a contributor who does have Xcode but a misconfigured selector.Testing
cargo fmt --all -- --checkRUSTFLAGS="-D warnings" cargo clippy --workspace --all-targets -- -D warningsRUSTFLAGS="-D warnings" cargo test --workspaceRUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --document-private-items --exclude openlogi-ui --exclude openlogi-desktop --exclude openlogi-overlay --exclude openlogi-agent.cargo/config.tomlis a workspace-wide build input.devenv.nixsets its ownDEVELOPER_DIRindependently (its own hardcoded path plus arequireXcodeMetalcheck that fails loudly with an actionable message when Metal isn't available), and.github/workflows/build.yml/release.ymlset their ownDEVELOPER_DIRenv at the workflow level — neither depends on the removed block, so devenv and CI builds are unaffected.Fixes #1079