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

qt5: Fallback to mktemp -d for temporary QT root. #28237

Merged
merged 2 commits into from
Sep 23, 2017

Conversation

avnik
Copy link
Contributor

@avnik avnik commented Aug 13, 2017

Fallback to mktemp -d for temporary QT root, if invoked in shell mode.
Should fix problem from #27174.

Motivation for this change

Folks often complains on qt5_nix dropped into source dir, when nix-shell -p qt5.qtbase invoked, so little hack was added, to fallback to old solution with mktemp -d in shell mode.

Thing to be done -- figure out, why this dir not properly removed on shell exit.

PS Tested against master, but PR to staging, to avoid unnessesary mass-rebuild.
(at least qastools and virtualbox builds with this patch applied).

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Fallback to `mktemp -d` for temporary QT root, if invoked in shell mode.
Should fix problem from NixOS#27174.
@mention-bot
Copy link

@avnik, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ttuegel to be a potential reviewer.

@Mic92
Copy link
Member

Mic92 commented Aug 27, 2017

Do we have a shell hook, which is called before nix-shell exits? Then we could clean the temporary directory properly.

@avnik
Copy link
Contributor Author

avnik commented Sep 10, 2017

@Mic92 I added some sort of exit hook, it works locally

@Mic92 Mic92 requested a review from ttuegel September 23, 2017 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants