GH-38902: [R] Handle failing library detection with pkg-config

[assignUser]

Rationale for this change

We can get into a broken state with a working test compile in nixlibs.R but empty PKG_LIBS when pkg-config fails to find some libraries (e.g. libcurl on mac due to missing system stubs) in configure. This leads to a failed test compile in configure with pc errors silenced.

What changes are included in this PR?

Catch this and rerun the pkg-config-less library detection that should fix this in most cases.

Are these changes tested?

locally and on cran (where this error first surfaced)

• Closes: [R] Include and Lib flags are missing on macOS #38902

[assignUser]

@github-actions crossbow submit -g r

[assignUser]

I have removed the --silence-errors from our pkg-config calls as a means to confirm the error on CRAN, it might be useful to keep them like that for easier debugging in the future as well (or silence them in the next release).

[assignUser]

A test run with an intentionally broken pkg-config https://github.com/assignUser/test-repo-a/actions/runs/7014022366/job/19120682558

[assignUser]

Looks like ${var// } is not portable afterall 😮‍💨, the failed pkg-config can cause whitespace to be in PKG_LIBS so we need to account for that (instead of just using -n/-z)k

[assignUser]

@github-actions crossbow submit test-r-* r-binary-packages

[github-actions]

Revision: 5db954a

Submitted crossbow builds: ursacomputing/crossbow @ actions-4d3caa5e57

Task	Status
r-binary-packages
test-r-arrow-backwards-compatibility
test-r-depsource-bundled
test-r-depsource-system
test-r-dev-duckdb
test-r-devdocs
test-r-gcc-11
test-r-gcc-12
test-r-install-local
test-r-install-local-minsizerel
test-r-library-r-base-latest
test-r-linux-as-cran
test-r-linux-rchk
test-r-linux-valgrind
test-r-minimal-build
test-r-offline-maximal
test-r-offline-minimal
test-r-rhub-debian-gcc-devel-lto-latest
test-r-rhub-debian-gcc-release-custom-ccache
test-r-rhub-ubuntu-gcc-release-latest
test-r-rstudio-r-base-4.1-opensuse153
test-r-rstudio-r-base-4.2-centos7-devtoolset-8
test-r-rstudio-r-base-4.2-focal
test-r-ubuntu-22.04
test-r-versions

[github-actions]

⚠️ GitHub issue #38902 has been automatically assigned in GitHub to PR creator.

[github-actions]

⚠️ GitHub issue #38902 has no components, please add labels for components.

[paleolimbot]

I tested with MacOS 11 with all "recipes" installed and get:

dewey@tidytuesday ~ % R CMD INSTALL ~/Desktop/arrow_14.0.0.tar.gz 
* installing to library ‘/Library/Frameworks/R.framework/Versions/4.3-x86_64/Resources/library’
* installing *source* package ‘arrow’ ...
** using staged installation
*** Found libcurl and OpenSSL >= 1.1
*** Successfully retrieved C++ binaries (darwin-x86_64-openssl-1.1)
*** Checksum validated successfully for libarrow binary: darwin-x86_64-openssl-1.1/arrow-14.0.0.zip
Package libcurl was not found in the pkg-config search path.
Perhaps you should add the directory containing `libcurl.pc'
to the PKG_CONFIG_PATH environment variable
Package 'libcurl', required by 'arrow', not found
Package libcurl was not found in the pkg-config search path.
Perhaps you should add the directory containing `libcurl.pc'
to the PKG_CONFIG_PATH environment variable
Package 'libcurl', required by 'arrow', not found
Package libcurl was not found in the pkg-config search path.
Perhaps you should add the directory containing `libcurl.pc'
to the PKG_CONFIG_PATH environment variable
Package 'libcurl', required by 'arrow', not found
*** pkg-config failed to find libcurl, but S3 is enabled. Running detection without pkg-config.

With the linking step giving me:

clang++ -arch x86_64 -std=gnu++17 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup -single_module -multiply_defined suppress -L/Library/Frameworks/R.framework/Resources/lib -L/opt/R/x86_64/lib -o arrow.so RTasks.o altrep.o array.o array_to_vector.o arraydata.o arrowExports.o bridge.o buffer.o chunkedarray.o compression.o compute-exec.o compute.o config.o csv.o dataset.o datatype.o expression.o extension-impl.o feather.o field.o filesystem.o io.o json.o memorypool.o message.o parquet.o r_to_arrow.o recordbatch.o recordbatchreader.o recordbatchwriter.o safe-call-into-r-impl.o scalar.o schema.o symbols.o table.o threadpool.o type_infer.o -L/private/var/folders/4v/hvphchw50373drst9g5smz1w0000gn/T/RtmpTxZLvI/R.INSTALL71103e64d60a/arrow/libarrow/arrow-14.0.0/lib -L/opt/R/x86_64/lib -larrow_acero -larrow_dataset -lparquet -larrow -larrow_bundled_dependencies -lcurl -lssl -lcrypto -lcurl -lssl -lcrypto -F/Library/Frameworks/R.framework/.. -framework R -Wl,-framework -Wl,CoreFoundation

This works! I see -lcurl -lssl -lcrypto -lcurl -lssl -lcrypto (i.e., flags show up twice), but it doesn't seem to give an error.

I will also try on 10.13 shortly!

[paleolimbot]

Ok, this works on 10.13 as well with the same output.

I think it is worth updating the log output that says Found libcurl and OpenSSL to be slightly more descriptive, since it is clearly not finding libcurl based on the very loud messages to the contrary that show up below.

Also, worth fixing the doubled -lcurl and friends for tidiness and to avoid raised eyebrows by viewers of the install log.

[jonkeane]

One question, one suggestions, and one thing that's probably much larger that we should look into as a follow on (IMHO)

[assignUser]

I think it is worth updating the log output that says Found libcurl and OpenSSL to be slightly more descriptive, since it is clearly not finding libcurl based on the very loud messages to the contrary that show up below.

Well it did find curl as it's using the headers... so maybe "Found headers for ..."?

Also, worth fixing the doubled -lcurl and friends for tidiness and to avoid raised eyebrows by viewers of the install log.

We could do something like sort -u but that would change the flag order and I'd rather not... probably better to change the logic.

[assignUser]

@github-actions crossbow submit r-binary-packages

[github-actions]

Revision: 950f1dd

Submitted crossbow builds: ursacomputing/crossbow @ actions-5371802dfe

Task	Status
r-binary-packages

[github-actions]

Revision: 80ecd01

Submitted crossbow builds: ursacomputing/crossbow @ actions-cad65ca65d

Task	Status
r-binary-packages

[assignUser]

This should be good to go now (the crossbow failure is due to #38779 )

[paleolimbot]

This looks good to me! Thank you!

[assignUser]

@github-actions crossbow submit r-binary-packages

rebased to fix crossbow

[github-actions]

Revision: fb676f8

Submitted crossbow builds: ursacomputing/crossbow @ actions-d33deb962e

Task	Status
r-binary-packages

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 29263a1.

There was 1 benchmark result indicating a performance regression:

• Commit Run on ursa-i9-9960x at 2023-12-02 00:19:21Z
  • tpch (R) with engine=arrow, format=parquet, language=R, memory_map=False, query_id=TPCH-02, scale_factor=1

The full Conbench report has more details. It also includes information about 5 possible false positives for unstable benchmarks that are known to sometimes produce them.