Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cc-wrapper: disable response files for ccache #178280

Merged
merged 1 commit into from
Apr 15, 2023

Conversation

veprbl
Copy link
Member

@veprbl veprbl commented Jun 19, 2022

The problem is that the ccache wrapper does not work with response
files (it seems like the fd doesn't survive after exec in the
makeWrapper wrapper).

with import ./. {};
  
let
  _ccacheStdenv = ccacheStdenv.override {
    extraConfig = ''
      export CCACHE_DIR=/tmp/ccache
      export CCACHE_UMASK=007
    '';
  };
in
  hello.override { stdenv = _ccacheStdenv; }
Description of changes

Resolves: #119779

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

The problem is that the ccache wrapper does not work with response
files (it seems like the fd doesn't survive after exec in the
makeWrapper wrapper).
@veprbl veprbl requested a review from Ericson2314 as a code owner June 19, 2022 21:41
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jun 19, 2022
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1089

Copy link
Member

@Atemu Atemu left a comment

Choose a reason for hiding this comment

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

Diff LGTM and this fixes the issue.

I'm inclined to merge this.

@Atemu Atemu requested review from kira-bruneau and r-burns April 15, 2023 10:29
@Atemu
Copy link
Member

Atemu commented Apr 15, 2023

Pinged ccache maintainers.

Copy link
Contributor

@kira-bruneau kira-bruneau left a comment

Choose a reason for hiding this comment

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

Oh thanks for the ping @Atemu! This LGTM!!

@kira-bruneau kira-bruneau merged commit 99a9508 into NixOS:master Apr 15, 2023
@kira-bruneau
Copy link
Contributor

kira-bruneau commented Apr 15, 2023

Oh wait I just noticed that it looks like ccache fixed parsing response files in v4.6.1: https://ccache.dev/releasenotes.html#_build_improvements_6 - this probably isn't needed anymore.

@Atemu
Copy link
Member

Atemu commented Apr 15, 2023

It wasn't working for me and the ccache version is 4.8 in my local tree I cherry-picked this onto.

@kira-bruneau
Copy link
Contributor

kira-bruneau commented Apr 15, 2023

Ok, thanks! Just wanted to check first if ccache added supported for it in the meantime.

@Atemu
Copy link
Member

Atemu commented Apr 15, 2023

@kira-bruneau Btw, where did you see clang response files being supported in that changelog?

@kira-bruneau
Copy link
Contributor

Oh it was Fixed parsing of MSVC response files. - but I'm assuming that would be different than the clang response files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clang response files break ccache
4 participants