fix: do not rely on which program during configuration

[szhorvat]

This PR avoids reliance on the which utility, which may not be present on all systems. It addresses complaints like this one: NixOS/nixpkgs#286406

It also renamed xml2_include_paths to lilbxml2_cflags to better reflect what this variable contains.

[aviator-app]

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.

---

See the real-time status of this PR on the Aviator webapp.

Use the Aviator Chrome Extension to see the status of your PR within GitHub.

[szhorvat]

Second commits adds libxml2 library detection in the configure script.

[krlmlr]

Thanks for taking this on.

[szhorvat]

R says,

possible bashism in configure line 5 ('command' with option other than -p):

even though -v is in the POSIX standard.

It is ironic that -p, the only flag they allow, doesn't actually work on macOS ...

[krlmlr]

🤷

The upstream issue just mentions that they need "which" as a package dependency, not that we must fix here.

[szhorvat]

@krlmlr I think the current version should work. It should be reasonably safe to try to run these commands (xml2-config and pkg-config) as a means of detecting their availability. Also committed the missing Makevars.in change. Please have another look.

Note that for the library detection I requested the flags required when libxml2 is a static library. I believe this won't hurt when it is actually a shared library, but it will allow using a static one as well.

If we only wanted shared ones, we'd need xml2-config --libs --dynamic and pkg-config --libs libxml-2.0.

[krlmlr]

Thanks!

[krlmlr]

The order change may or may not be relevant. Let's see.

[szhorvat]

@Antonov548 Do we actually need libxml2_cflags="" at the beginning of the configure script? If not, let's remove it. If yes, can you explain why, and perhaps also add libxml2_libs=""?

[Antonov548]

@Antonov548 Do we actually need libxml2_cflags="" at the beginning of the configure script? If not, let's remove it. If yes, can you explain why, and perhaps also add libxml2_libs=""?

Don't really need it anymore #1235
I think it was needed when parsing of cflags was in configure script.

[PhDyellow]

Thanks for noticing the Nix PR! I think Nix is a great ecosystem for reproducible research and I am actively using it with R, but it does require updates as upstream packages change. Nix also has a very different way of approaching dependencies than most operating systems and package managers.

https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/r-modules/default.nix contains most of the nix-specific tweaks we make to ensure R packages have the right system-level dependencies and settings. R package dependencies are automatically extracted from CRAN.

I opened NixOS/nixpkgs#286406 because the error message was cryptic. If the build had immediately errored with "which - not found on system" the R ecosystem maintainers would have known exactly what to do. Once the next igraph version hits CRAN, the build will break anyway because glpk hasn't been added as an igraph system dependency in Nix yet, but the error message is clear and will be quickly fixed. PRs welcome too.

Of course, removing which as a dependency doesn't hurt.