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

Fix build hook error for libstore library users #8517

Merged

Conversation

roberth
Copy link
Member

@roberth roberth commented Jun 14, 2023

Motivation

A library shouldn't require changes to the caller's argument handling, especially if it doesn't have to, and indeed we don't have to.

This changes the lookup order to prioritize the hardcoded path to nix if it exists. The static executable still finds itself through /proc and the like.

Context

Encountered in hercules-ci-agent when run on a single user installation.

A solution is also required for #7735. It currently contains a workaround, whereas this is a general solution.

nixos-option probably also suffers from this problem.

May be obsoleted by #1221, but a simple bugfix should take priority I think.

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@roberth roberth added bug store Issues and pull requests concerning the Nix store labels Jun 14, 2023
@github-actions github-actions bot added with-tests Issues related to testing. PRs with tests have some priority and removed store Issues and pull requests concerning the Nix store labels Jun 14, 2023
@Ericson2314

This comment was marked as resolved.

@roberth roberth force-pushed the fix-build-hook-error-for-lib-users branch 2 times, most recently from 93667d5 to 4cbd60f Compare June 14, 2023 23:14
tests/test-libstoreconsumer/local.mk Outdated Show resolved Hide resolved
tests/test-libstoreconsumer/test-libstoreconsumer Outdated Show resolved Hide resolved
src/libstore/globals.cc Show resolved Hide resolved
@roberth roberth force-pushed the fix-build-hook-error-for-lib-users branch 3 times, most recently from b231749 to c75784e Compare June 14, 2023 23:37
A library shouldn't require changes to the caller's argument handling,
especially if it doesn't have to, and indeed we don't have to.

This changes the lookup order to prioritize the hardcoded path to nix
if it exists. The static executable still finds itself through /proc
and the like.
@roberth roberth force-pushed the fix-build-hook-error-for-lib-users branch from c75784e to d2696cd Compare June 15, 2023 12:32
@fricklerhandwerk
Copy link
Contributor

Triaged in Nix team meeting:

@edolstra edolstra merged commit 7138361 into NixOS:master Jun 16, 2023
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2023-06-16-nix-team-meeting-minutes-63/29316/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug with-tests Issues related to testing. PRs with tests have some priority
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants