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

gamin: Fix cross compiling issues #96903

Merged
merged 1 commit into from Oct 20, 2020
Merged

gamin: Fix cross compiling issues #96903

merged 1 commit into from Oct 20, 2020

Conversation

@kampka
Copy link
Contributor

@kampka kampka commented Sep 1, 2020

Motivation for this change

Gamin fails to build cross-platform because it tries to run
AC_RUN_IFELSE during configure which fails as the test program is built
for the wrong platform and cannot execute. Since the test is only for
the 'abstract socket namespace' feature, we can just pin the results
for out builds, it is 'yes' for Linux and 'no' for Darwin (and other
BSD).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@kampka
Copy link
Contributor Author

@kampka kampka commented Sep 1, 2020

@GrahamcOfBorg build gamin
@GrahamcOfBorg build pkgsCross.aarch64-multiplatform.gamin

@jonringer
Copy link
Contributor

@jonringer jonringer commented Sep 1, 2020

- [have_abstract_sockets=no])
-AC_LANG_POP(C)
-AC_MSG_RESULT($have_abstract_sockets)
+have_abstract_sockets=yes
Copy link
Member

@matthewbauer matthewbauer Sep 1, 2020

you should be able to use $host_os and AC_CANONICAL_HOST to get the right value here, avoiding the need for two patches

Copy link
Contributor Author

@kampka kampka Sep 2, 2020

Good point. I am not very familiar with the whole autotools toolschain, so I appreciate reviews like these 👍
The build already has $target_os which should be good for this case, though I cannot test it against Darwin.

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Sep 1, 2020

Thanks for the contribution. Is there something specific you need gamin for? I would rather just drop this package since upstream is dead and we would have to maintain our fork here.

@kampka
Copy link
Contributor Author

@kampka kampka commented Sep 2, 2020

Thanks for the contribution. Is there something specific you need gamin for? I would rather just drop this package since upstream is dead and we would have to maintain our fork here.

No, it just comes up in the dependency chain of my configuration, probably through the gnome2 reference in openjdk via gvfs.
A quick search shows that gamin is still referenced by a couple of packages. I don't know how feasible it is to remove it short term.

Gamin fails to build cross-platform because it tries to run
AC_RUN_IFELSE during configure which fails as the test program is built
for the wrong platform and cannot execute. Since the test is only for
the 'abstract socket namespace' feature, we can just pin the results
for out builds, it is 'yes' for Linux and 'no' for Darwin (and other
BSD).
@kampka
Copy link
Contributor Author

@kampka kampka commented Sep 2, 2020

@GrahamcOfBorg build gamin
@GrahamcOfBorg build pkgsCross.aarch64-multiplatform.gamin

@kampka kampka requested a review from matthewbauer Sep 2, 2020
@FRidh FRidh added this to WIP in Staging via automation Sep 4, 2020
@FRidh FRidh moved this from WIP to Needs review in Staging Sep 4, 2020
@FRidh FRidh merged commit 507a5e9 into NixOS:staging Oct 20, 2020
23 checks passed
Staging automation moved this from Needs review to Done Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Staging
  
Done
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants