Skip to content

Comments

libstore: Make sure that initNix has been called#7478

Merged
thufschmitt merged 1 commit intoNixOS:masterfrom
hercules-ci:make-sure-initNix-called
Jan 2, 2023
Merged

libstore: Make sure that initNix has been called#7478
thufschmitt merged 1 commit intoNixOS:masterfrom
hercules-ci:make-sure-initNix-called

Conversation

@roberth
Copy link
Member

@roberth roberth commented Dec 19, 2022

Prevent bugs like cachix/cachix#477

call upon the programmer to handle this correctly. However, we only add
this in a key locations, so as not to litter the code. */
void assertLibStoreInitialized();

Copy link
Member Author

Choose a reason for hiding this comment

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

I would have done this simply at the initNix level, but the library layering does not allow this.

Perhaps the common parts of initialization should be moved into libutil?

@roberth roberth force-pushed the make-sure-initNix-called branch from 1817cbc to aba6eb3 Compare December 24, 2022 13:41
@thufschmitt thufschmitt merged commit fb8fc6f into NixOS:master Jan 2, 2023
@fricklerhandwerk
Copy link
Contributor

Discussed in Nix team meeting 2022-12-23:

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2022-12-23-nix-team-meeting-minutes-19/24400/1

@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Jan 3, 2023

Discussed in Nix team meeting 2023-01-02:

  • @fricklerhandwerk: please add the rationale to the commit message, otherwise it's not clear why the change was made
    • @roberth: we're trying to prevent using the SQLite in the wrong way. if we don't load the config file, it will use the default settings
    • @thufschmitt: could we instead use the dirty static variable trick we have in other places, so the code is run whenever the library is loaded?
    • @edolstra: what we actually want is that the Config object is initialised, and that's what we should assert somewhere
      • @roberth: opened an issue to follow up and improve the factoring
        • @edolstra: this thing is not designed to be used by anybody else, this is where all the problems come from. so there is much to be improved
  • decision: merge

@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-01-02-nix-team-meeting-minutes-20/24403/1

Ma27 added a commit to Ma27/nix that referenced this pull request Jan 28, 2023
Since NixOS#7478 it's mandatory that `initNix()` is called for store
operations. However that's not the case when running `openStore()` in
Perl using the perl-bindings. That breaks e.g. `hydra-eval-jobset` when
built against Nix 2.13 which uses small portions of the store API.

The `-lnixmain` is necessary to make sure that
`$out/lib/perl5/site_perl/5.36.0/x86_64-linux-thread-multi/auto/Nix/Store/Store.so`
is linked against `libnixmain.so` which exposes the `initNix` symbol.
Ma27 added a commit to Ma27/nix that referenced this pull request Jan 28, 2023
Since NixOS#7478 it's mandatory that `initNix()` is called for store
operations. However that's not the case when running `openStore()` in
Perl using the perl-bindings. That breaks e.g. `hydra-eval-jobset` when
built against Nix 2.13 which uses small portions of the store API.

The `-lnixmain` is necessary to make sure that
`$out/lib/perl5/site_perl/5.36.0/x86_64-linux-thread-multi/auto/Nix/Store/Store.so`
is linked against `libnixmain.so` which exposes the `initNix` symbol.
@Ma27
Copy link
Member

Ma27 commented Jan 28, 2023

FYI this appears to break the perl bindings that are used by e.g. Hydra. Fix is in #7705.

Ma27 added a commit to Ma27/nix that referenced this pull request Feb 2, 2023
Since NixOS#7478 it's mandatory that `initNix()` is called for store
operations. However that's not the case when running `openStore()` in
Perl using the perl-bindings. That breaks e.g. `hydra-eval-jobset` when
built against Nix 2.13 which uses small portions of the store API.

The `-lnixmain` is necessary to make sure that
`$out/lib/perl5/site_perl/5.36.0/x86_64-linux-thread-multi/auto/Nix/Store/Store.so`
is linked against `libnixmain.so` which exposes the `initNix` symbol.
Ma27 added a commit to Ma27/nix that referenced this pull request Feb 2, 2023
Since NixOS#7478 it's mandatory that `initLibStore()` is called for store
operations. However that's not the case when running `openStore()` in
Perl using the perl-bindings. That breaks e.g. `hydra-eval-jobset` when
built against Nix 2.13 which uses small portions of the store API.
Ma27 added a commit to Ma27/nix that referenced this pull request Feb 2, 2023
Since NixOS#7478 it's mandatory that `initLibStore()` is called for store
operations. However that's not the case when running `openStore()` in
Perl using the perl-bindings. That breaks e.g. `hydra-eval-jobset` when
built against Nix 2.13 which uses small portions of the store API.
github-actions bot pushed a commit that referenced this pull request Feb 3, 2023
Since #7478 it's mandatory that `initLibStore()` is called for store
operations. However that's not the case when running `openStore()` in
Perl using the perl-bindings. That breaks e.g. `hydra-eval-jobset` when
built against Nix 2.13 which uses small portions of the store API.

(cherry picked from commit 51013da)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants