DOI-USGS / ISIS3

Integrated Software for Imagers and Spectrometers v3. ISIS3 is a digital image processing software package to manipulate imagery collected by current and past NASA and International planetary missions.
https://isis.astrogeology.usgs.gov
Other
200 stars 168 forks source link

Nixpkgs build: Did not recognize UNIX operating system! #5657

Open arunoruto opened 1 week ago

arunoruto commented 1 week ago

Description
Since we had some problems with conda here and there at work, I took it upon myself to package ISIS using Nix. At the beginning I had the problem, that it did not recognize my system, due to packages being build inside a sandbox. This is because the builds need to be reproducable and should not depend on outside factors. I was able to bypass this by running the build command with the --option sandbox false flag and sudo. I was wondering if it would be possible to provide a cmake flag with the OS type, instead of letting the script figure it out. Or in other words: if the flag isn't set, let the script figure it out, but if is, then take that value.

I also didn't understand quote well why some specifics of the system are needed in that regard. Can someone elaborate on it?

Example
Maybe check for CMAKE_SYSTEM_VERSION beforehand and give examples on how the string should look like.

On another note, if someone wants, they can try out my build using the nix command:

# if flakes are enabled
sudo nix build github:arunoruto/flake#isis -L --option sandbox false # --experimental-features 'nix-command flakes'
# if flakes are not enabled, which would be default for most systems usually
sudo nix --experimental-features 'nix-command flakes' build github:arunoruto/flake#isis -L --option sandbox false
Kelvinrr commented 1 week ago

Im not familiar with NIX but I can try to help out. Some questions:

At the beginning I had the problem, that it did not recognize my system, due to packages being build inside a sandbox. This is because the builds need to be reproducable and should not depend on outside factors. I was able to bypass this by running the build command with the --option sandbox false flag and sudo. I was wondering if it would be possible to provide a cmake flag with the OS type, instead of letting the script figure it out. Or in other words: if the flag isn't set, let the script figure it out, but if is, then take that value.

Do you mean to force CMAKE to build for a different system than it is defaulting to? In which you might want to look at cmake's docs on it. Although Im not sure it's a good idea. Why do you need to overwrite the OS? Does NIX not have a built in mechanism for this? Since different OSes would need to package binaries differently.

I also didn't understand quote well why some specifics of the system are needed in that regard. Can someone elaborate on it?

What specifics are you talking about? CMAKE uses the OS to change a lot of behavior since Windows, Linux and Mac all do things fairly differently.

arunoruto commented 1 week ago

Hey!

Do you mean to force CMAKE to build for a different system than it is defaulting to? In which you might want to look at cmake's docs on it. Although Im not sure it's a good idea. Why do you need to overwrite the OS? Does NIX not have a built in mechanism for this? Since different OSes would need to package binaries differently.

I am talking about this part here. It checks for the contacts of /etc/lsb-release or /etc/os-release, but this isn't allowed in Nix. It would be nice to be able to specify these things via a flag. If I am not mistaken, the end result for me was something along the line of Linux_NixOS_20_05_Ukari.

What specifics are you talking about? CMAKE uses the OS to change a lot of behavior since Windows, Linux and Mac all do things fairly differently. I do understand that differentiating between Linux, Mac and Windows is important, but where does it come into play, if it is a Debian, RedHat, or other Linux system?

For Nix, you get the correct compiled packages from the resolver, so I do not have to take care of what gcc or cmake is being used. I can change my dependencies or other things depending on the system type (stdenv.isLinux, stdenv.isDarwin (Mac), stdenv.isBSD, ...) and architecture (stdenv.isAarch64, stdenv.isAarch32, stdenv.isx86_64, ...). I would probably differentiate only between isLinux and isDarwin and set the additional flag to make it aware if the target is a Linux or Mac. I guess the CMAKE_SYSTEM_NAME flag would not change the behaviour of get_os_version?

Kelvinrr commented 1 week ago

What exactly do you mean that it's "not allowed"? theoretically, you should be able to add some code to that function that does another check to see if the OS is Nix

arunoruto commented 1 week ago

What exactly do you mean that it's "not allowed"? theoretically, you should be able to add some code to that function that does another check to see if the OS is Nix

While the package is being built/compiled, it is "put" in a sandbox. It is unaware of its surroundings, so it can not depend on files outside of the sandbox. You can guide the process with certain variables, like if some additional dependencies should be included for a system type or an architecture, but that is about it. Scripts and build tools are meant to be provided what they are build against and not let them figure it out themselves. It does sound a bit cumbersome, but it is for the sake of reproducibility. The famous "it works on my machine" means it works on all supported machines. Like a docker container, just without the container.

Long story short, it would be nice to have a flag which would overwrite this part here: https://github.com/DOI-USGS/ISIS3/blob/437e334fb8b74ccf768b07573bd03de761478de9/isis/cmake/Utilities.cmake#L90-L185 and assign the final value of get_os_version to the flags input. If it isn't provided, the usual checks would still come into play.

Kelvinrr commented 1 week ago

I guess my point is in my last message is that if you have unique knowledge on how to get the package built for nixpkgs, why not contribute the change yourself? Mostly likely a simple enough change. You would be more suited to make that contribution. Also, unless someone else says otherwise, this isn't a priority for this project right now.