zeehio / MassSpecWavelet

MassSpecWavelet Bioconductor package
8 stars 2 forks source link

BioC build error: DLL requires the use of native symbols #5

Closed sneumann closed 1 year ago

sneumann commented 1 year ago

Hi, on both Windows and Linux there is a build error, resulting from two vignettes failing to run:

Error: processing vignette 'FindingLocalMaxima.Rmd' failed with diagnostics:
DLL requires the use of native symbols

This sounds like https://stat.ethz.ch/pipermail/r-package-devel/2023q1/009016.html which was fixed like this:

MassSpecWavelet is initialising C code in https://github.com/zeehio/MassSpecWavelet/blob/409e048483dee944f7f2495c741d1f3cf2c266c9/src/init.c#L10

In MassSpecWavelet the calls are via strings: https://github.com/zeehio/MassSpecWavelet/blob/409e048483dee944f7f2495c741d1f3cf2c266c9/R/localMaximum.R#L98 and https://github.com/zeehio/MassSpecWavelet/blob/409e048483dee944f7f2495c741d1f3cf2c266c9/R/findLocalMaxWinSize.R#L21

Yours, Steffen

bryanhanson commented 1 year ago

I am seeing this as well, and CRAN is threatening to archive my ChemoSpec package as a result. Can this fix be merged and pushed to Bioconductor? Pretty please?

zeehio commented 1 year ago

A VERY BIG THANK YOU to both of you for the issue and the pull request with the fix.

My father passed away last week and I was not able to see this until today. Focusing beyond basic reviewing, merging and pushing is still hard, so your detailed and kind messages helped A LOT. Both @sneumann with the issue and the patch and @bryanhanson with the reminder, and including the advice of pushing to Bioconductor. Given my current situation I might have forgotten.

To the topic: I already pushed to bioc devel. Since RELEASE_3_16 is already frozen and the issue applies to R-4.3 and above, I believe pushing to devel should be enough.

Bioconductor may take some time (24-48h?) to build and publish the result.

imatge

Thanks again

sneumann commented 1 year ago

Hi, I can confirm that MassSpecWavelet is back to all-green, and so are xcms and CAMERA. Thanks for pulling that in, Yours, Steffen