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

Nix flake updates #365

Merged
merged 8 commits into from
Feb 19, 2025
Merged

Nix flake updates #365

merged 8 commits into from
Feb 19, 2025

Conversation

cadkin
Copy link
Contributor

@cadkin cadkin commented Feb 9, 2025

Split out of #364.

Specifically deals with the changes to the build configuration and Nix files.

  • Cleans up Nix files to use more standard callPackage pattern.
  • Keeps multiple versions of adamantine and dealii for usage.
    • Going forward, it might make more sense to remove these as users can directly ask for versions based on available git tags (e.g. nix run github:adamantine-sim/adamantine/1.0).
    • However, this requires that a Nix flake is available in whatever revision the tag points to, so until v1.0 is completely obsolete we should probably keep the .versions schema.
  • source/CMakeLists.txt now defines adamantine as a object library to avoid un-necessary linking.
  • application/CMakeLists.txt now specifies an install, so make install works, useful for Nix tooling.

@cadkin cadkin changed the title Nix Flake Updates Nix flake updates Feb 9, 2025
@wd15
Copy link
Contributor

wd15 commented Feb 12, 2025

So I am currently reviewing the PR. How do I get a local version using nix shell? I ran nix shell in the working copy and got

$ nix shell                                                                                                                                            
error:                                                                                                                                                                                         
       … while calling the 'derivationStrict' builtin                                                                                                                                          
                                                                                                                                                                                               
         at /builtin/derivation.nix:9:12: (source not available)                                                                                                                               
                                                                                                                                                                                               
       … while evaluating the derivation attribute 'name'                                                                                                                                      
                                                                                                                                                                                               
         at /nix/store/awsvw44jla0idziiks2zwgzslfd2dczn-source/pkgs/stdenv/generic/make-derivation.nix:336:7:                                                                                  
                                                                                                                                                                                               
          335|     // (optionalAttrs (attrs ? name || (attrs ? pname && attrs ? version)) {                                                                                                    
          336|       name =                
       error: attribute 'dirtyShortRev' missing                                                

       at /nix/store/s5grjgm5pn7kqmldncbgg6hr2zgw89sn-source/flake.nix:50:23:

           49|           devel = callPackage ./nix/adamantine/common.nix {
           50|             version = self.dirtyShortRev;                                       
             |                       ^         
           51|             src     = self;                    

This functionality currently works on master (for me at least).

nix shell github:adamantine-sim/adamantine works, but I'm assuming that's pulling in the current master flake.

Also, nix shell github:cadkin/adamantine?ref=nix-flake gives the same error as above. Any ideas on this? Thanks!

@cadkin
Copy link
Contributor Author

cadkin commented Feb 12, 2025

Ah, that was my bad. I was only referencing the dirty revision when I should have been referencing both versions. I just pushed a fix, we can discuss during our meeting later.

@wd15
Copy link
Contributor

wd15 commented Feb 12, 2025

nix shell github:cadkin/adamantine?ref=nix-flake builds now! However, getting the following though

$ nix shell github:cadkin/adamantine?ref=nix-flake#adamantine.versions.stable
[1/2/3 built, 0.0 MiB DL] building adamantine-1.0 (buildPhase):

...

error: builder for '/nix/store/p176dm3jkrmns35ygncgi1w5d0k0hjcz-adamantine-1.0.drv' failed with exit code 2;                                                                                   
       last 10 log lines:                                                                                                                                                                      
       > In static member function 'static std::char_traits<char>::char_type* std::char_traits<char>::copy(char_type*, const char_type*, std::size_t)',                                        
       >     inlined from 'std::streamsize boost::iostreams::basic_gzip_decompressor<Alloc>::peekable_source<Source>::read(char*, std::streamsize) [with Source = boost::iostreams::detail::lin
ked_streambuf<char, std::char_traits<char> >; Alloc = std::allocator<char>]' at include/boost/iostreams/filter/gzip.hpp:582:56,                                                                
       >     inlined from 'static typename boost::iostreams::int_type_of<T>::type boost::iostreams::detail::read_device_impl<boost::iostreams::input>::get(T&) [with T = boost::iostreams::basic_gzip_decompressor<>::peekable_source<boost::iostreams::detail::linked_streambuf<char, std::char_traits<char> > >]' at include/boost/iostreams/read.hpp:172:29,
       >     inlined from 'typename boost::iostreams::int_type_of<T>::type boost::iostreams::get(T&) [with T = basic_gzip_decompressor<>::peekable_source<detail::linked_streambuf<char, std::char_traits<char> > >]' at include/boost/iostreams/read.hpp:43:42,
       >     inlined from 'std::streamsize boost::iostreams::basic_gzip_decompressor<Alloc>::read(Source&, char_type*, std::streamsize) [with Source = boost::iostreams::detail::linked_streambuf<char, std::char_traits<char> >; Alloc = std::allocator<char>]' at include/boost/iostreams/filter/gzip.hpp:508:46:
       > /nix/store/62qjb50708fdhb4f2y7zxyqr1afir4fk-gcc-13.3.0/include/c++/13.3.0/bits/char_traits.h:435:56: warning: 'void* __builtin_memcpy(void*, const void*, long unsigned int)' accessing 9223372036854775808 or more bytes at offsets 0 and [-9223372036854775808, 9223372036854775807] may overlap up to 9223372036854775809 bytes at offset -1 [8;;https://gcc.gnu.org/onlinedocs/gc
c/Warning-Options.html#index-Wrestrict-Wrestrict8;;]
       >   435 |         return static_cast<char_type*>(__builtin_memcpy(__s1, __s2, __n));
       >       |                                        ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
       > make[1]: *** [CMakeFiles/Makefile2:169: source/CMakeFiles/Adamantine.dir/all] Error 2
       > make: *** [Makefile:91: all] Error 2
       For full logs, run 'nix-store -l /nix/store/p176dm3jkrmns35ygncgi1w5d0k0hjcz-adamantine-1.0.drv'.
error: builder for '/nix/store/p176dm3jkrmns35ygncgi1w5d0k0hjcz-adamantine-1.0.drv' failed with exit code 2;
       last 10 log lines:
       > In static member function 'static std::char_traits<char>::char_type* std::char_traits<char>::copy(char_type*, const char_type*, std::size_t)',
       >     inlined from 'std::streamsize boost::iostreams::basic_gzip_decompressor<Alloc>::peekable_source<Source>::read(char*, std::streamsize) [with Source = boost::iostreams::detail::lin
ked_streambuf<char, std::char_traits<char> >; Alloc = std::allocator<char>]' at include/boost/iostreams/filter/gzip.hpp:582:56,
       >     inlined from 'static typename boost::iostreams::int_type_of<T>::type boost::iostreams::detail::read_device_impl<boost::iostreams::input>::get(T&) [with T = boost::iostreams::basi
c_gzip_decompressor<>::peekable_source<boost::iostreams::detail::linked_streambuf<char, std::char_traits<char> > >]' at include/boost/iostreams/read.hpp:172:29,
       >     inlined from 'typename boost::iostreams::int_type_of<T>::type boost::iostreams::get(T&) [with T = basic_gzip_decompressor<>::peekable_source<detail::linked_streambuf<char, std::c
har_traits<char> > >]' at include/boost/iostreams/read.hpp:43:42,
   
...

    last 10 log lines:
       > In static member function 'static std::char_traits<char>::char_type* std::char_traits<char>::copy(char_type*, const char_type*, std::size_t)',
       >     inlined from 'std::streamsize boost::iostreams::basic_gzip_decompressor<Alloc>::peekable_source<Source>::read(char*, std::streamsize) [with Source = boost::iostreams::detail::linked_streambuf<char, std::char_traits<char> >; Alloc = std::allocator<char>]' at include/boost/iostreams/filter/gzip.hpp:582:56,
       >     inlined from 'static typename boost::iostreams::int_type_of<T>::type boost::iostreams::detail::read_device_impl<boost::iostreams::input>::get(T&) [with T = boost::iostreams::basic_gzip_decompressor<>::peekable_source<boost::iostreams::detail::linked_streambuf<char, std::char_traits<char> > >]' at include/boost/iostreams/read.hpp:172:29,
       >     inlined from 'typename boost::iostreams::int_type_of<T>::type boost::iostreams::get(T&) [with T = basic_gzip_decompressor<>::peekable_source<detail::linked_streambuf<char, std::char_traits<char> > >]' at include/boost/iostreams/read.hpp:43:42,
       >     inlined from 'std::streamsize boost::iostreams::basic_gzip_decompressor<Alloc>::read(Source&, char_type*, std::streamsize) [with Source = boost::iostreams::detail::linked_streambuf<char, std::char_traits<char> >; Alloc = std::allocator<char>]' at include/boost/iostreams/filter/gzip.hpp:508:46:
       > /nix/store/62qjb50708fdhb4f2y7zxyqr1afir4fk-gcc-13.3.0/include/c++/13.3.0/bits/char_traits.h:435:56: warning: 'void* __builtin_memcpy(void*, const void*, long unsigned int)' accessing 9223372036854775808 or more bytes at offsets 0 and [-9223372036854775808, 9223372036854775807] may overlap up to 9223372036854775809 bytes at offset -1 [-Wrestrict]
       >   435 |         return static_cast<char_type*>(__builtin_memcpy(__s1, __s2, __n));
       >       |                                        ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
       > make[1]: *** [CMakeFiles/Makefile2:169: source/CMakeFiles/Adamantine.dir/all] Error 2
       > make: *** [Makefile:91: all] Error 2
       For full logs, run 'nix-store -l /nix/store/p176dm3jkrmns35ygncgi1w5d0k0hjcz-adamantine-1.0.drv'.

@cadkin
Copy link
Contributor Author

cadkin commented Feb 13, 2025

Interesting... I did find a fault with my code where the hash was incorrect for dealii 9.5.2 and it caused the build to fail before it ever got to that point, so I'm curious how it even got to the build phase for you. After fixing that hash and building (force push incoming in a bit), it built for me fine.

Those logs that you posted only seem to show warnings, so I don't think any of that output is the actual error there. If I had to guess, two things might have caused your build to fail:

  • It was using dealii 9.6.2 due to the hash mismatch and that caused it to fail due to the incompatibilities that Bruno was talking about.
  • Your build ran out of memory and the kernel OOM killer got you.

Try again with my hash fix and see if that fixes it. If not, we can dive into it a bit more.

@cadkin
Copy link
Contributor Author

cadkin commented Feb 13, 2025

I also went ahead and updated the docs with instructions for how to use the cache. Let me know if it makes sense or not.

@wd15
Copy link
Contributor

wd15 commented Feb 13, 2025

nix shell github:cadkin/adamantine?ref=nix-flake#adamantine.versions.stable builds! Nice work. However, nix flake check doesn't like pkgs and libs as they aren't derivations.

warning: unknown flake output 'config'
error:
       … while checking flake output 'packages'

         at /nix/store/01x5k4nlxcpyd85nnr0b9gm89rm8ff4x-source/lib.nix:43:9:

           42|       // {
           43|         ${key} = (attrs.${key} or { }) // {
             |         ^
           44|           ${system} = ret.${key};

       … while checking the derivation 'packages.x86_64-linux.pkgs'

         at /nix/store/qdl836lvr3hkyysrzjl15vjkdhc5vqxj-source/flake.nix:43:14:

           42|       inherit libs;
           43|       inherit pkgs;
             |              ^
           44|

       error: flake attribute 'packages.x86_64-linux.pkgs' is not a derivation

Removing those gives

warning: Git tree '/home/wd15/git/adamantine' is dirty
warning: unknown flake output 'config'
warning: unknown flake output 'nixConfig'
warning: The check omitted these incompatible systems: aarch64-darwin, aarch64-linux, x86_64-darwin
Use '--all-systems' to check all.

which I guess is fine. Whether nix flake check is correct on this determination is another matter. Your choice!

@wd15
Copy link
Contributor

wd15 commented Feb 13, 2025

Also, I put together a small github action and it's failing for mac os x. I'll drop os x for now, but maybe it should be revisited in the future. It's currently not failing for a compile build failure, but it will be a can of worms.

@cadkin
Copy link
Contributor Author

cadkin commented Feb 13, 2025

nix shell github:cadkin/adamantine?ref=nix-flake#adamantine.versions.stable builds! Nice work. However, nix flake check doesn't like pkgs and libs as they aren't derivations.

warning: unknown flake output 'config'
error:
       … while checking flake output 'packages'

         at /nix/store/01x5k4nlxcpyd85nnr0b9gm89rm8ff4x-source/lib.nix:43:9:

           42|       // {
           43|         ${key} = (attrs.${key} or { }) // {
             |         ^
           44|           ${system} = ret.${key};

       … while checking the derivation 'packages.x86_64-linux.pkgs'

         at /nix/store/qdl836lvr3hkyysrzjl15vjkdhc5vqxj-source/flake.nix:43:14:

           42|       inherit libs;
           43|       inherit pkgs;
             |              ^
           44|

       error: flake attribute 'packages.x86_64-linux.pkgs' is not a derivation

Removing those gives

warning: Git tree '/home/wd15/git/adamantine' is dirty
warning: unknown flake output 'config'
warning: unknown flake output 'nixConfig'
warning: The check omitted these incompatible systems: aarch64-darwin, aarch64-linux, x86_64-darwin
Use '--all-systems' to check all.

which I guess is fine. Whether nix flake check is correct on this determination is another matter. Your choice!

Hmm, I've never messed too much with flake checks. I can remove the libs and the pkgs sets since they were really just for ease of testing (I think I'll just put them in another 'unknown' output so we can still access them for now; we should upstream those libs to nixpkgs eventually anyways).

If I recall correctly, the main point of contention is that it only checks the builtin schema with no customization point for specifying your schema. For example, nixConfig means something to nix develop and the like but apparently not to nix flake check. There's actually a PR on Nix itself to add support for custom schemas so maybe we could integrate that to check for those attributes when that gets merged.

Also, I put together a small github action and it's failing for mac os x. I'll drop os x for now, but maybe it should be revisited in the future. It's currently not failing for a compile build failure, but it will be a can of worms.

Oh, apparently gdb is marked as incompatible with MacOS. If I just move it to the 'linux-only' part of the shell attributes, it should work again. I'll add that to those other fixes.

@wd15
Copy link
Contributor

wd15 commented Feb 13, 2025

Is the cache for mdfbaam up to date with current head (on this branch)? I'm running the github action using the install-nix action, which is supposed to add the trusted user to the nix config (I'm assuming this is good enough to get the cache). However, it doesn't seem to be pulling in the cached binaries, but rebuilding everything slowly.

The Cachix Action has the ability to pull from Cachix as well if the magic config in the flake doesn't work, but it should work.

@wd15
Copy link
Contributor

wd15 commented Feb 13, 2025

I can get cached binaries with,

          extra_nix_config: |
            trusted-public-keys = mdfbaam.cachix.org-1:WCQinXaMJP7Ny4sMlKdisNUyhcO2MHnPoobUef5aTmQ= cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
            substituters = https://mdfbaam.cachix.org https://cache.nixos.org

which is fine so don't worry.

@cadkin
Copy link
Contributor Author

cadkin commented Feb 14, 2025

Ah, I had the attribute in the wrong place... it's not an output but rather a top-level attribute. Fixed!

@cadkin
Copy link
Contributor Author

cadkin commented Feb 14, 2025

I just went ahead and built 1.0 and the current tip of my branch, so both of these (plus dependencies) should be able to be pulled from the cache now.

@wd15
Copy link
Contributor

wd15 commented Feb 14, 2025

I added a PR into your nix-flake branch for the github action.

@wd15
Copy link
Contributor

wd15 commented Feb 14, 2025

@Rombur: I'm totally happy with this PR at this point

@wd15
Copy link
Contributor

wd15 commented Feb 14, 2025

One thing. Are the tests actually being run on the build?

@cadkin
Copy link
Contributor Author

cadkin commented Feb 14, 2025

One thing. Are the tests actually being run on the build?

I knew I was forgetting something... added integration_2d for all builds and merged in your PR.

It might be worth our time to integrate the tests via nix since all tests are being built either way - it's just a matter of removing the checkPhase to make them all run (getting them actually all passing is another story). Probably better left for another PR.

@wd15
Copy link
Contributor

wd15 commented Feb 14, 2025

One thing. Are the tests actually being run on the build?

I knew I was forgetting something... added integration_2d for all builds and merged in your PR.

It might be worth our time to integrate the tests via nix since all tests are being built either way - it's just a matter of removing the checkPhase to make them all run (getting them actually all passing is another story). Probably better left for another PR.

For sure. Leave it for another PR. Let's get this merged if @Rombur is ok with it. As long as one test runs that's fine.

@cadkin cadkin force-pushed the nix-flake branch 2 times, most recently from 2105c1d to 3ee6458 Compare February 18, 2025 01:06
Should make targets like `make install` work now.
Copy link
Member

@Rombur Rombur left a comment

Choose a reason for hiding this comment

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

Looks good thanks. I'll merge the PR once the workflow tests are done

@Rombur
Copy link
Member

Rombur commented Feb 18, 2025

The macos build failed. I don't why there is an issue with macos but not for ubuntu

@cadkin
Copy link
Contributor Author

cadkin commented Feb 18, 2025

Interesting... seems unhappy with the test for some reason, something related to MPI? It's hard to tell without the full logs.

I can just guard the tests with a not-macos check and avoid running them there so at least it compiles. Or we can just remove the mac test all together? What do you think @wd15?

@cadkin
Copy link
Contributor Author

cadkin commented Feb 18, 2025

Maybe related: NixOS/nixpkgs#376951

Edit: just pushed up a bump to the nixpkgs version since the fix for this has been backported, let's see if that resolves the darwin build.

Also fixes some dealii build warnings
@wd15
Copy link
Contributor

wd15 commented Feb 18, 2025

I noticed that the macos build is still building the derivations and not pulling from Cachix. The ubuntu build doesn't seem to have that issue.

@cadkin
Copy link
Contributor Author

cadkin commented Feb 18, 2025

I noticed that the macos build is still building the derivations and not pulling from Cachix. The ubuntu build doesn't seem to have that issue.

Yeah, that's expected. So far I've been manually populating that cache whenever I push a build up. I only have Linux computers so I haven't been building and caching derivations for macos.

It would be good to setup a pipeline that builds closures and caches for linux/darwin/x86/arm eventually.

@Rombur Rombur merged commit 087317a into adamantine-sim:master Feb 19, 2025
3 checks passed
@wd15
Copy link
Contributor

wd15 commented Feb 19, 2025

I noticed that the macos build is still building the derivations and not pulling from Cachix. The ubuntu build doesn't seem to have that issue.

Yeah, that's expected. So far I've been manually populating that cache whenever I push a build up. I only have Linux computers so I haven't been building and caching derivations for macos.

It would be good to setup a pipeline that builds closures and caches for linux/darwin/x86/arm eventually.

Actually, that's just another step in the GitHub Action, you'd have to add your private Cachix key to the GitHub Action secrets. That action would have to run on the central repository only and wouldn't run on a pull-request so two slightly different workflows would be required I think that run on different triggers. I have one for another project that does it. Will port it over and ping you at some point.

@wd15
Copy link
Contributor

wd15 commented Feb 19, 2025

@cadkin @Rombur: appreciate both of your efforts on this. It's a big help for my Digital Twin AM work.

@Rombur Rombur mentioned this pull request Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants