Makefile: fix ar flags

[barracuda156]

On macOS the current code may produce a broken static library due to missing table of contents – because ranlib is not being run. As a substitute for that, s can be added to ar flags.

[barracuda156]

Otherwise I am getting this when installing BoomSpikeSlab:

/opt/local/bin/g++-mp-13 -std=gnu++17 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup -single_module -multiply_defined suppress -L/opt/local/Library/Frameworks/R.framework/Resources/lib -Wl,-headerpad_max_install_names -L/opt/local/lib -lMacportsLegacySupport -Wl,-rpath,/opt/local/lib/libgcc -arch ppc -o BoomSpikeSlab.so boom_spike_slab_init.o logit_spike_slab_wrapper.o mlm_spike_slab_wrapper.o nested_regression_wrapper.o nnet_wrapper.o poisson_spike_slab_wrapper.o probit_spike_slab_wrapper.o quantile_spike_wrapper.o shrinkage_regression_wrapper.o spike_slab_wrapper.o splines.o /opt/local/Library/Frameworks/R.framework/Versions/4.3/Resources/library/Boom/lib/libboom.a -F/opt/local/Library/Frameworks/R.framework/.. -framework R -Wl,-framework -Wl,CoreFoundation
ld: in /opt/local/Library/Frameworks/R.framework/Versions/4.3/Resources/library/Boom/lib/libboom.a, archive has no table of contents
collect2: error: ld returned 1 exit status
make: *** [BoomSpikeSlab.so] Error 1
ERROR: compilation failed for package ‘BoomSpikeSlab’

After running ranlib on libboom.a, everything works fine, and tests for BoomSpikeSlab pass. I am not sure why Boom installation succeeds despite a broken lib.

[steve-the-bayesian]

This was caught by CRAN for Boom_0.9.13, and was fixed in Boom_0.9.14 (the next day). If you're still having this problem with 0.9.14 please let me know.

…

On Mon, Dec 18, 2023 at 1:01 PM Sergey Fedorov ***@***.***> wrote: On macOS the current code may produce a broken static library due to missing table of contents – because ranlib is not being run. As a substitute for that, s can be added to ar flags. ------------------------------ You can view, comment on, or merge this pull request online at: #79 Commit Summary - 634893e <634893e> Makefile: fix ar flags File Changes (1 file <https://github.com/steve-the-bayesian/BOOM/pull/79/files>) - *M* Makefile <https://github.com/steve-the-bayesian/BOOM/pull/79/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52> (2) Patch Links: - https://github.com/steve-the-bayesian/BOOM/pull/79.patch - https://github.com/steve-the-bayesian/BOOM/pull/79.diff — Reply to this email directly, view it on GitHub <#79>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABMVDVMN6TQVJDMXLZO3CA3YKCVJJAVCNFSM6AAAAABA2EB6OGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGA2DONBUG4ZTCMQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[barracuda156]

@steve-the-bayesian Thank you for responding. I actually got the error with 0.9.14. CRAN tarball for it still seems to have rc without s and without ranlib.

[steve-the-bayesian]

Here is the AR line from Boom 0.9.10 libboom.a: ${OBJECTS} ${AR} rc $@ $^ # strip $@ This matches the line in 0.9.14 and is the method required by CRAN. It builds and links on both my mac and linux machines. I don't think the 's' option or ranlib are needed.

…

On Mon, Dec 18, 2023 at 1:34 PM Sergey Fedorov ***@***.***> wrote: @steve-the-bayesian <https://github.com/steve-the-bayesian> Thank you for responding. I actually got the error with 0.9.14. CRAN tarball for it still seems to have rc without s and without ranlib. — Reply to this email directly, view it on GitHub <#79 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABMVDVKCQEU56BDOFITFXUDYKCZETAVCNFSM6AAAAABA2EB6OGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRRG4YDQMJSGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[barracuda156]

@steve-the-bayesian It is not needed on every platform, but it is needed on some.

See, for example, this issue: PolMine/RcppCWB#80 (comment)
And commit that fixed it: PolMine/RcppCWB@c6df1cc

[barracuda156]

Builds fine with updated ar flags on every macOS: https://ports.macports.org/port/R-Boom/details

[steve-the-bayesian]

Sergey, does this need to be the CRAN version? I build R packages for Boom and related packages (including bsts) locally using scripts located in the BOOM/install directory. I could add a 'macports' flag that would instruct the script to build the library however you like, and then we wouldn't have CRAN's heavy portability rules as a constraint. Have a look at .../install/create_boom_rpackage and let me know if that approach fits your needs.

…

On Tue, Dec 19, 2023 at 10:48 PM Sergey Fedorov ***@***.***> wrote: Builds fine with updated ar flags on every macOS: https://ports.macports.org/port/R-Boom/details — Reply to this email directly, view it on GitHub <#79 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABMVDVM7ZBLRRH2SCRVVEWTYKKC4FAVCNFSM6AAAAABA2EB6OGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRTHEZTSNBVGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[barracuda156]

@steve-the-bayesian Well, if the point is just fixing it for Macports, that has been done already locally. However, that leaves the build broken for anyone who tries to build it on affected platforms without Macports. Unfortunately, it does not even fail as such, but leaves the library unusable, which can be frustrating for someone who is not familiar with the issue.

Of course, if there is a known case where adding s breaks the library for some other platform, it should be avoided in a general case. On macOS it is fine across the board, this is pretty sure. I cannot guarantee the same for every possible Unix-like OS, but it seems it is universally supported.

CRAN's heavy portability rules as a constraint

While I am aware that CRAN normally does not bother about anything besides a few bleeding edge versions, I do not think they have a policy of breaking something out of principle :)
Having said that, this is of course up to you to decide whether better portability matters or not. IMO, the fix is trivial and does not introduce any complications in terms of future efforts to support it.

[steve-the-bayesian]

Gotcha. I'll run it by CRAN. If they don't object I'll keep the change. If they do (and they're full of surprises) then I'm open to other solutions.

…

On Wed, Dec 20, 2023 at 11:00 AM Sergey Fedorov ***@***.***> wrote: @steve-the-bayesian <https://github.com/steve-the-bayesian> Well, if the point is just fixing it for Macports, that has been done already locally. However, that leaves the build broken for anyone who tries to build it on affected platforms without Macports. Unfortunately, it does not even fail as such, but leaves the library unusable, which can be frustrating for someone who is not familiar with the issue. Of course, if there is a known case where adding s breaks the library for some other platform, it should be avoided in a general case. On macOS it is fine across the board, this is pretty sure. I cannot guarantee the same for every possible Unix-like OS, but it seems it is universally supported. CRAN's heavy portability rules as a constraint While I am aware that CRAN normally does not bother about anything besides a few bleeding edge versions, I do not think they have a policy of breaking something out of principle :) Having said that, this is of course up to you to decide whether better portability matters or not. IMO, the fix is trivial and does not introduce any complications in terms of future efforts to support it. — Reply to this email directly, view it on GitHub <#79 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABMVDVKOLM5OJ73DQLX3EQLYKMYUTAVCNFSM6AAAAABA2EB6OGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRUHE4DQMZSG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[barracuda156]

Thank you!