Bioconductor / Biostrings

Efficient manipulation of biological strings
https://bioconductor.org/packages/Biostrings
54 stars 16 forks source link

S.h seems to be deprecated #66

Closed tillea closed 2 years ago

tillea commented 2 years ago

Hi, the Biostrings Debian package received a bug report about

gcc -I"/usr/share/R/include" -DNDEBUG  -I'/usr/lib/R/site-library/S4Vectors/include' -I'/usr/lib/R/site-library/IRanges/include' -I'/usr/lib/R/site-library/XVector/include'    -fpic  -g -O2 -ffile-prefix-map=/build/r-base-g6byQC/r-base-4.1.3.20220405=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2  -c BitMatrix.c -o BitMatrix.o
BitMatrix.c:9:10: fatal error: S.h: No such file or directory
    9 | #include <S.h> /* for Salloc() */
      |          ^~~~~

I've found this hint how this can be fixed and I guess you are aware of this. Kind regards, Andreas.

LiNk-NY commented 2 years ago

Hi Andreas, @tillea Note that this has been fixed here: https://github.com/Bioconductor/Biostrings/commit/c3340745870a88d1558e093a7f892c1aeac784ef

hpages commented 2 years ago

@tillea And most importantly, it seems that Debian is building the wrong version of Bioconductor. The S.h file was removed from R 4.2 and they are reporting a bug for Biostrings 2.62.0, which belongs to Bioconductor 3.14 (the current release). BioC 3.14 should only be used with R 4.1: it has been expressly designed and tested against this version of R.

If you are using R 4.2, you should build Bioconductor 3.15, which is still in development and will be released on April 27. Biostrings is currently at version 2.63.3. This version belongs to BioC 3.15 and works with R 4.2.

Best, H.

tillea commented 2 years ago

Hi Hervé,

Am Wed, Apr 13, 2022 at 09:46:59AM -0700 schrieb Hervé Pagès:

@tillea And most importantly, it seems that Debian is building the wrong version of Bioconductor. The S.h file was removed from R 4.2 and they are reporting a bug for Biostrings 2.62.0, which belongs to Bioconductor 3.14 (the current release). BioC 3.14 should only be used with R 4.1: it has been expressly designed and tested against this version of R.

Thanks a lot for this hint. The R-pkg team is always heading for the latest stable release of any R / BioC package. The maintainer of r-core obviously does the same but without any warning about bumping to a potentially incompatible R version. (Debian has the distribution "experimental" for this kind of migrations but it was not used unfortunately.)

If you are using R 4.2, you should build Bioconductor 3.15, which is still in development and will be released on April 27. Biostrings is currently at version 2.63.3. This version belongs to BioC 3.15 and works with R 4.2.

I'm afraid we simply need to wait for Bioconductor 3.15 in this case since there are more Bioconductor packages with this issue and trying to patch all these and assuming that everything will work as expected is probably nanaive. This will cause some noise amongst all Bioconductor packages which will removed from the testing distribution. Users of Debian stable will not be affected.

Kind regards, Andreas.

eddelbuettel commented 2 years ago

BioC 3.14 should only be used with R 4.1: it has been expressly designed and tested against this version of R.

Debian simply does not have a facility for this type of 'pinning', which makes the bi-annual BioConductor release cycle a (regular and recurring) challenge. Debian builds (in the lingo of some other places) 'at @HEAD' and all current packages are expected to play together (not unlike CRAN).

Which is why R 4.2.0 snapshots are in unstable to maximize the test surface -- and which is what we've done for 20+ years with these r-base packages (unless we are in a freeze for a Debian release in which thewhen they go into experimental where they effectively remain mostly untested).

So this never was a BioC bug.

tillea commented 2 years ago

Hi again Hervé,

@tillea And most importantly, it seems that Debian is building the wrong version of Bioconductor. The S.h file was removed from R 4.2 and they are reporting a bug for Biostrings 2.62.0, which belongs to Bioconductor 3.14 (the current release). BioC 3.14 should only be used with R 4.1: it has been expressly designed and tested against this version of R. If you are using R 4.2, you should build Bioconductor 3.15, which is still in development and will be released on April 27. Biostrings is currently at version 2.63.3. This version belongs to BioC 3.15 and works with R 4.2.

I intended to upgrade groHMM to version 1.29.1 which has remaining references to S.h

$ grep -R S.h src/MLEfit.c:#include <S.h> src/DecayAlgorithm.c:#include <S.h> src/hmmMiscFunctions.c:#include <S.h> src/hmmFwBw.c:#include <S.h> src/hmmViterbi.c:#include <S.h> src/hmmEM.c:#include <S.h> src/Windowing.c:#include <S.h>

Am I missing something? Should I use the groHMM Github issue tracke?

Kind regards Andreas.

nileshpatra commented 2 years ago

Hi all,

I intended to upgrade groHMM to version 1.29.1 which has remaining references to S.h $ grep -R S.h src/MLEfit.c:#include <S.h> src/DecayAlgorithm.c:#include <S.h> src/hmmMiscFunctions.c:#include <S.h> src/hmmFwBw.c:#include <S.h> src/hmmViterbi.c:#include <S.h> src/hmmEM.c:#include <S.h> src/Windowing.c:#include <S.h> Am I missing something?

I have fixed this in our debian package, weirdly enough, it includes new R.h and S.h without any conditionals for including either. (I'd expect a #if/#ifdef on R version for includes) -- even tests pass locally for me.

This seems like an oversight at upstream's end.

Should I use the groHMM Github issue tracker?

The authors of groHMM and biostrings atleast as listed on bioconductor are not the same, so this is sort of obvious :) In anycase, we should be good now.

hpages commented 2 years ago

Right, I'm not the maintainer of groHMM, but, FWIW, I see that the latest stable version of groHMM (1.30.1, part of BioC 3.15) was fixed last week (i.e. no more #include <S.h>). Official URL to the latest version of groHMM in the current BioC release: https://bioconductor.org/packages/groHMM

Cheers, H.