Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multiple definition of `strncasecmp' with the new toolschain #36

Closed
sneumann opened this issue Apr 4, 2016 · 47 comments
Closed

multiple definition of `strncasecmp' with the new toolschain #36

sneumann opened this issue Apr 4, 2016 · 47 comments

Comments

@sneumann
Copy link
Owner

sneumann commented Apr 4, 2016

Hi, currently we have build issues on the windows machine of the build farm
for both http://bioconductor.org/checkResults/devel/bioc-LATEST/mzR/moscato2-buildsrc.html
and http://bioconductor.org/spb_reports/mzR_2.5.5.1_buildreport_20160404074007.html

Somehow, two the *.o from both *.c files (rnetCDF.c and R_init_mzR.o) include
the symbols from the stdlib, thus linking them results in multiple definition ofstrncasecmp'`.
I tried both the ULDAR and standard linker, both give the same issue. I failed to set up
a local working development on Windows, so I can't help much here.

Yours,
Steffen

@kasperdanielhansen
Copy link

Have the static libraries in src/win been rebuild using the new toolchain?

@dtenenba
Copy link
Contributor

dtenenba commented Apr 7, 2016

It is not too hard to set up a windows build environment, the instructions are at:

https://github.com/Bioconductor/Bioconductor/blob/master/documentation/new-toolchain-setup.md

I attempted to update the static libraries in src/win, to answer @kasperdanielhansen 's question. But the maintainers of mzR should try it themselves.

I don't think making a change and then trying it in the single package builder is an efficient way to figure this out. A working build environment is required. @sneumann , please let me know how your attempt to set it up failed and I will help you.

@dtenenba
Copy link
Contributor

I worked on this a bit. I actually got mzR to install on i386 only. But had problems with x64, it seems to be the libnetcdf.a that is the problem. I tried both the original version that was there and also the one from local323.zip. It seems like perhaps this library needs to be rebuilt from source under the new toolchain?

@dtenenba
Copy link
Contributor

Is there any documentation anywhere about how that libnetcdf.a file was built and how to rebuild it?

I don't have much more time to work on this...so I hope someone else can fix it. Otherwise our release will not include the proteomics stack on windows.

@lgatto
Copy link
Collaborator

lgatto commented Apr 11, 2016

I think libnetcdf.a stem from the olden days; netcdf support was originally available in xcms and was later moved to mzR - @sneumann, you probably know.

@kasperdanielhansen
Copy link

Could we get a bit more history here... Various R packages uses NetCDF and
I am guessing that at least some of them work on Windows. Any reason for
not using this other stuff, or was it just not around when you made mzR or
? Seems to me that the sensible start is to figure out how other windows
packages uses libnetcdf and try to incorporate this.

On Mon, Apr 11, 2016 at 3:52 PM, Laurent Gatto notifications@github.com
wrote:

I think libnetcdf.a stem from the olden days; netcdf support was
originally available in xcms and was later moved to mzR - @sneumann
https://github.com/sneumann, you probably know.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#36 (comment)

@dtenenba
Copy link
Contributor

I think that ncdf4 is the main R interface to netcdf.

@kasperdanielhansen
Copy link

Like on CRAN they use netcdf 4, and here is the package on the CRAN
winbuilder:
http://win-builder.r-project.org/GPLcompliance/
and some notes from Ripley
http://www.stats.ox.ac.uk/pub/Rtools/libs.html

Any reason for not starting with those; I assume Rnetcdf and ncdf4 (the two
CRAN packages with NetCDF) builds their support for NetCDF statically?

On Mon, Apr 11, 2016 at 4:04 PM, Kasper Daniel Hansen <
kasperdanielhansen@gmail.com> wrote:

Could we get a bit more history here... Various R packages uses NetCDF and
I am guessing that at least some of them work on Windows. Any reason for
not using this other stuff, or was it just not around when you made mzR or
? Seems to me that the sensible start is to figure out how other windows
packages uses libnetcdf and try to incorporate this.

On Mon, Apr 11, 2016 at 3:52 PM, Laurent Gatto notifications@github.com
wrote:

I think libnetcdf.a stem from the olden days; netcdf support was
originally available in xcms and was later moved to mzR - @sneumann
https://github.com/sneumann, you probably know.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#36 (comment)

@sneumann
Copy link
Owner Author

Hi Kasper, you guessed right, as Laurent hinted, the embedded libnetcdf
comes from xcms around 2006 or 2007. Back then I cross-compiled
from Linux. See [1] for a historic post :-)

I just had a quick look at ncdf4, it has libnetcdf as SystemRequirements:

Yours,
Steffen

[1] https://groups.google.com/forum/#!topic/xcms-devel/fVdB85GxuZg

@dtenenba
Copy link
Contributor

Yes, they use the libnetcdf.a from spatial324.zip, and both those packages install from source just fine on moscato2.

The above zip file is unzipped in c:/local323 and the following is defined in
$R_HOME/etc/$R_ARCH/Makeconf:

NETCDF = c:/local323�

��

@kasperdanielhansen
Copy link

So why don't we try to use this one. Need some work on the Windows makevars file.

Best,
Kasper
(Sent from my phone.)

On Apr 11, 2016, at 16:16, dtenenba notifications@github.com wrote:

Yes, they use the libnetcdf.a from spatial324.zip, and both those packages install from source just fine on moscato2.

The above zip file is unzipped in c:/local323 and the following is defined in
$R_HOME/etc/$R_ARCH/Makeconf:

NETCDF = c:/local323

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@dtenenba
Copy link
Contributor

Hoping more eyes can look at this. @mtmorgan @jimhester

@kasperdanielhansen
Copy link

Im trying to get a virtual machine up and running, but its slow going.

Kasper

On Mon, Apr 11, 2016 at 9:25 PM, dtenenba notifications@github.com wrote:

Hoping more eyes can look at this. @mtmorgan https://github.com/mtmorgan
@jimhester https://github.com/jimhester


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#36 (comment)

@jimhester
Copy link

I rebuilt ntcdf with the new toolchain using sources from http://www.stats.ox.ac.uk/pub/Rtools/goodies/sources/ and moved libnetcdf.a from that build to src/win/{x64,i386} which allows x64 builds to pass R CMD check on my machine, but the i386 builds are failing to load with the following error

** testing if installed package can be loaded
Error : .onLoad failed in loadNamespace() for 'mzR', details:
  call: value[[3L]](cond)
  error: failed to load module Ramp from package mzR
cannot allocate vector of length 1790997466
Error: loading failed
Execution halted
ERROR: loading failed

Not sure what is going wrong, @dtenenba if you want to try with the recompiled libraries they are available at https://github.com/rwinlib/netcdf

@dtenenba
Copy link
Contributor

Hi @jimhester , that sounds like progress!

I was able to get i386 to install but not x64, I get the same error as in the title of this issue (still).

Can you spell out exactly what you did to get x64 to install? Here is what I tried:

git clone https://github.com/sneumann/mzR.git
git clone https://github.com/rwinlib/netcdf.git
cd mzR/src/win
cp ../../../netcdf/include/* i386/
cp ../../../netcdf/include/* x64/
cp ../../../netcdf/lib-4.9.3/i386/* i386/
cp ../../../netcdf/lib-4.9.3/x64/* x64/
# copied the version of libpwiz.a that I built earlier for x64 to mzR/src/win/x64
cd ../..
vi DESCRIPTION # set version to 2.5.6
cd ..
R CMD build --no-build-vignettes mzR
R --arch x64 CMD INSTALL --build --no-multiarch mzR_2.5.6.tar.gz

Since I got i386 to install, and you got x64 to install, I am hoping that if I can do the latter, we might have solved this....
Thanks.

@dtenenba
Copy link
Contributor

@jimhester Maybe you can email me your src/win/x64/libpwiz.a ? Then if that works we can discuss how you built that and make sure the process is documented.

@jimhester
Copy link

@dtenenba I used the SVN mzR (and included libpwiz.a)

@dtenenba
Copy link
Contributor

@jimhester I did a fresh checkout from svn and copied over the netcdf files. I am still getting the same results with the x64 installation attempt. The i386 install still works though.

Would still be helpful I think to see exactly what you are doing. Thanks.

@jimhester
Copy link

OK I just tried from fresh windows 10 VM and got the same results as I did previously

  1. Download and install windows 10 from (https://www.microsoft.com/en-us/evalcenter/evaluate-windows-10-enterprise)
  2. Install R-3.3.0beta from https://cran.r-project.org/bin/windows/base/R-3.3.0beta-win.exe
  3. Install Rtools33 from https://cran.r-project.org/bin/windows/Rtools/Rtools33.exe
  4. Install Bioconductor / mzR dependencies (in R console) and download mzR
source("https://bioconductor.org/biocLite.R")
biocLite(c("Rcpp", "Biobase", "BiocGenerics", "ProtGenerics", "msdata", "RUnit", "mzID", "BiocStyle", "knitr"))
download.file("https://bioconductor.org/packages/3.3/src/contrib/mzR_2.5.5.tar.gz", "mzR_2.5.5.tar.gz")
  1. Build 32 bit 'C:\Program Files\R\R-3.3.0beta\bin\i386\R.exe' CMD check .\mzR_2.5.5.tar.gz --no-build-vignettes --no-manual Still fails with same error as above when trying to install.
  2. Build 64 bit 'C:\Program Files\R\R-3.3.0beta\bin\x64\R.exe' CMD check .\mzR_2.5.5.tar.gz --no-build-vignettes --no-manual runs without error.

I used --no-build-vignettes and --no-manual as I didn't install MiKTex on this VM.
It looks like you can't use --force-multiarch because of a non-empty configure.win in mzR, so I had to run the checks separately.

Note I did not copy any libraries in this case, but from the previous attempt I don't think it will help the error seen.

@kasperdanielhansen
Copy link

How about environment variables like path

Best,
Kasper
(Sent from my phone.)

On Apr 13, 2016, at 10:55, Jim Hester notifications@github.com wrote:

OK I just tried from fresh windows 10 VM and got the same results as I did previously

Download and install windows 10 from (https://www.microsoft.com/en-us/evalcenter/evaluate-windows-10-enterprise)
Install R-3.3.0beta from https://cran.r-project.org/bin/windows/base/R-3.3.0beta-win.exe
Install Rtools33 from https://cran.r-project.org/bin/windows/Rtools/Rtools33.exe
Install Bioconductor / mzR dependencies (in R console) and download mzR
source("https://bioconductor.org/biocLite.R")
biocLite(c("Rcpp", "Biobase", "BiocGenerics", "ProtGenerics", "msdata", "RUnit", "mzID", "BiocStyle", "knitr"))
download.file("https://bioconductor.org/packages/3.3/src/contrib/mzR_2.5.5.tar.gz", "mzR_2.5.5.tar.gz")
Build 32 bit 'C:\Program Files\R\R-3.3.0beta\bin\i386\R.exe' CMD check .\mzR_2.5.5.tar.gz --no-build-vignettes --no-manual Still fails with same error as above when trying to install.
Build 64 bit 'C:\Program Files\R\R-3.3.0beta\bin\x64\R.exe' CMD check .\mzR_2.5.5.tar.gz --no-build-vignettes --no-manual runs without error.
I used --no-build-vignettes and --no-manual as I didn't install MiKTex on this VM.
It looks like you can't use --force-multiarch because of a non-empty configure.win in mzR, so I had to run the checks separately.

Note I did not copy any libraries in this case, but from the previous attempt I don't think it will help the error seen.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@jimhester
Copy link

Only environment variable I set was adding C:/Rtools/bin to the PATH. I did not have to set BINPREF

@dtenenba
Copy link
Contributor

@jimhester I can reproduce your results exactly on a fresh Win10 VM (except I also had to install zlibbioc).

Unfortunately I still get the same results on moscato2, but maybe @kasperdanielhansen is right that somehow the same DLL is on the PATH more than once, or it could be some other variable. I will look into it.
I also tried reinstalling all of mzR's dependencies (now that CRAN is providing binaries built under the new toolchain). No difference.
I did have c:/Rtools/bin on my PATH, both on the VM and on moscato2.
Though BINPREF already comes set correctly (except for that ? that causes problems).

I usually don't use --force-multiarch; --merge-multiarch usually does the right thing in these cases.

@dtenenba
Copy link
Contributor

OK, I was able to get x64 to build, by temporarily reverting to the original etc/(i386/x64)/Makeconf files.
Now I have to figure out what it is in those files that is causing the issue. One obvious guess would be the NETCDF variable. But I can't just remove that from the Makeconf files, because other Bioconductor packages depend on netcdf. Maybe the libnetcdf.a just needs to be removed from mzR, but I tried that before and it did not work.

So, progress, but I'm not done, because mzR does not build in a vacuum, I have to get it to coexist with everything else that builds on moscato2.

I'll send an update when I have sorted this out. Thanks all for your work on this.

@dtenenba
Copy link
Contributor

OK, rather than mess around with the settings on moscato2 which is a production build machine about to start a build, I figured out how to reproduce the problem on my VM. Hope some of you (particularly @jimhester ) can try these steps and help me figure out what is going on.

  • Download local323.zip
  • Download spatial324.zip
  • Create a c:\local323 directory
  • Change to that directory and unzip local323.zip
  • Edit $R_HOME/etc/x64/Makeconf and change the LOCAL_SOFT line so it reads:
LOCAL_SOFT = c:/local323
  • Now install mzR, it should still install successfully, which tells us nothing in local323.zip is a problem:
"c:\Program Files\r\R-3.3.0beta\bin\R.exe" --arch x64 CMD INSTALL --no-multiarch mzR_2.5.5.tar.gz
  • In c:/local323, unzip spatial324.zip.
  • Now try again to install mzR, using the command line above.

For me it fails with

c:/Rtools/mingw_64/bin/g++ -shared -s -static-libgcc -o mzR.dll tmp.def cramp.o ramp_base64.o ramp.o RcppRamp.o RcppRampModule.o rnetCDF.o RcppPwiz.o RcppPwizModule.o RcppIdent.o RcppIdentModule.o R_init_mzR.o rampR.o -Wl,--dynamic-linker=/ld.exe -L./win//x64 -lws2_32 -lnetcdf -lpwiz -Lc:/local323/lib/x64 -Lc:/local323/lib -LC:/PROGRA~1/r/R-33~1.0BE/bin/x64 -lR
R_init_mzR.o:R_init_mzR.c:(.text+0x0): multiple definition of `strncasecmp'
rnetCDF.o:rnetCDF.c:(.text+0x0): first defined here
R_init_mzR.o:R_init_mzR.c:(.text+0x10): multiple definition of `strcasecmp'
rnetCDF.o:rnetCDF.c:(.text+0x10): first defined here
R_init_mzR.o:R_init_mzR.c:(.text+0x20): multiple definition of `_findfirst64i32'
rnetCDF.o:rnetCDF.c:(.text+0x20): first defined here
R_init_mzR.o:R_init_mzR.c:(.text+0xa0): multiple definition of `_findnext64i32'
rnetCDF.o:rnetCDF.c:(.text+0xa0): first defined here
R_init_mzR.o:R_init_mzR.c:(.text+0x120): multiple definition of `_fstat64i32'
rnetCDF.o:rnetCDF.c:(.text+0x120): first defined here
R_init_mzR.o:R_init_mzR.c:(.text+0x1b0): multiple definition of `_stat64i32'
rnetCDF.o:rnetCDF.c:(.text+0x1b0): first defined here
R_init_mzR.o:R_init_mzR.c:(.text+0x240): multiple definition of `fstat'
rnetCDF.o:rnetCDF.c:(.text+0x240): first defined here
collect2.exe: error: ld returned 1 exit status
no DLL was created
ERROR: compilation failed for package 'mzR'

Please see the external software page for a list of what libraries and headers are in spatial324.zip. Which one of those is causing the conflict, and how do we change mzR to avoid the conflict?

Answering my own question a bit, I did the following:

c:
cd local323\include
mkdir tmp
mv netcdf* tmp

After doing that, I can successfully install mzR. But that is not really a good option because it breaks other packages that depend on netcdf. Doing the converse (removing the netcdf header from mzR/src/win/x64) does not fix the problem. Neither does removing libnetcdf.a from the same place.

As for the i386 failure on the VM, it turns out I can reproduce that as well on moscato2 when I use the 'stock' etc/i386/Makeconf. I guess the problem is that 32-bit R on windows cannot allocate more than (a total of) 3GB of memory. Maybe we don't need to look this gift horse in the mouth though. Anyway, let's deal with one architecture at a time.

Thanks again
Dan

@kasperdanielhansen
Copy link

So reading the various comments on the toolchain, it seems to me that
Ripley was responsible for the current pre-4.6.3 tool chain and someone
else (group) was responsible for the new pre-4.9.3 toolchain. The
spatial343.zip file comes from stats.ox so I assume it was created by
Ripley. My guess is that the Netcdf library contained in this zip file
(and probably everything else) was build using the old tool chain and that
may be related to the problem we see. At this point, perhaps it would be
worthwhile to get in touch with the CRAN people and ask what they are doing
for netcdf for their two package ncdf4 and Rnetcdf. I don't know how good
contact we have with those folks?

Best,
Kasper

On Wed, Apr 13, 2016 at 8:05 PM, dtenenba notifications@github.com wrote:

OK, rather than mess around with the settings on moscato2 which is a
production build machine about to start a build, I figured out how to
reproduce the problem on my VM. Hope some of you (particularly @jimhester
https://github.com/jimhester ) can try these steps and help me figure
out what is going on.

LOCAL_SOFT = c:/local323

  • Now install mzR, it should still install successfully, which tells
    us nothing in local323.zip is a problem:

"c:\Program Files\r\R-3.3.0beta\bin\R.exe" --arch x64 CMD INSTALL --no-multiarch mzR_2.5.5.tar.gz

In c:/local323, unzip spatial324.zip.

Now try again to install mzR, using the command line above.

For me it fails with

c:/Rtools/mingw_64/bin/g++ -shared -s -static-libgcc -o mzR.dll tmp.def cramp.o ramp_base64.o ramp.o RcppRamp.o RcppRampModule.o rnetCDF.o RcppPwiz.o RcppPwizModule.o RcppIdent.o RcppIdentModule.o R_init_mzR.o rampR.o -Wl,--dynamic-linker=/ld.exe -L./win//x64 -lws2_32 -lnetcdf -lpwiz -Lc:/local323/lib/x64 -Lc:/local323/lib -LC:/PROGRA1/r/R-331.0BE/bin/x64 -lR
R_init_mzR.o:R_init_mzR.c:(.text+0x0): multiple definition of strncasecmp' rnetCDF.o:rnetCDF.c:(.text+0x0): first defined here R_init_mzR.o:R_init_mzR.c:(.text+0x10): multiple definition ofstrcasecmp'
rnetCDF.o:rnetCDF.c:(.text+0x10): first defined here
R_init_mzR.o:R_init_mzR.c:(.text+0x20): multiple definition of _findfirst64i32' rnetCDF.o:rnetCDF.c:(.text+0x20): first defined here R_init_mzR.o:R_init_mzR.c:(.text+0xa0): multiple definition of_findnext64i32'
rnetCDF.o:rnetCDF.c:(.text+0xa0): first defined here
R_init_mzR.o:R_init_mzR.c:(.text+0x120): multiple definition of _fstat64i32' rnetCDF.o:rnetCDF.c:(.text+0x120): first defined here R_init_mzR.o:R_init_mzR.c:(.text+0x1b0): multiple definition of_stat64i32'
rnetCDF.o:rnetCDF.c:(.text+0x1b0): first defined here
R_init_mzR.o:R_init_mzR.c:(.text+0x240): multiple definition of `fstat'
rnetCDF.o:rnetCDF.c:(.text+0x240): first defined here
collect2.exe: error: ld returned 1 exit status
no DLL was created
ERROR: compilation failed for package 'mzR'

Please see the external software
http://www.stats.ox.ac.uk/pub/Rtools/libs.html page for a list of what
libraries and headers are in spatial324.zip. Which one of those is causing
the conflict, and how do we change mzR to avoid the conflict?

Answering my own question a bit, I did the following:

c:
cd local323\include
mkdir tmp
mv netcdf* tmp

After doing that, I can successfully install mzR. But that is not really a
good option because it breaks other packages that depend on netcdf. Doing
the converse (removing the netcdf header from mzr/src/win/x64) does not
fix the problem. Neither does removing libnetcdf.a from the same place.

As for the i386 failure on the VM, it turns out I can reproduce that as
well on moscato2 when I use the 'stock' etc/i386/Makeconf. I guess the
problem is that 32-bit R on windows cannot allocate more than (a total of)
3GB of memory. Maybe we don't need to look this gift horse in the mouth
though. Anyway, let's deal with one architecture at a time.

Thanks again
Dan


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#36 (comment)

@dtenenba
Copy link
Contributor

You are correct about the different people responsible for the different toolchains.

However, many system dependencies that worked with the old toolchain should, and in fact, do work with the new toolchain. It seems that all system dependencies that have only C code in them will work without being rebuilt (though I have found the following exceptions, sometimes just for a specific sub-architecture: gsl and tiff).

The netcdf stuff in spatial324.zip works just fine with ncdf4 and Rnetcdf, I can build and check both of those packages under the new toolchain. I really think the problem has to do with the fact that mzR comes with its own netcdf library and header file which should not be necessary, and it causes the duplicate symbol error we see. I think it just needs a tweak to its Makevars.win but I haven't hit on the right one.

@kasperdanielhansen
Copy link

ah, now I understand a bit better. You're saying the problem arises
because you have spatial324 installed on your system, and having that
installed and then installing mzR creates some kind of conflict between
spatial324::netcdf and mzR:netcdf, right?

In that case, and using the historical info above, we should really just
modify mzR to drop its bundled netcdf and use spatial324::netcdf instead.
That way, we only have one copy to maintain on windows (and we also get rid
of some potetnial problematic license issues with bundling netcdf without
having the source code available).

This seems like it should be relatively easy, since we can peak at (say)
ncdf4 to see how it gets linked to the copy in spatial324. We would also
need to make sure that it statically incorporates the spatial324::netcdf in
the binary, since I assume that users of ncdf4 doesn't have to install
spatial324, right?

I might be able to devote some time to this over the weekend; I'm super
slowed down by the fact that I use windows for 1h once every 2 years or so.

Best,
Kasper

On Wed, Apr 13, 2016 at 10:46 PM, dtenenba notifications@github.com wrote:

You are correct about the different people responsible for the different
toolchains.

However, many system dependencies that worked with the old toolchain
should, and in fact, do work with the new toolchain. It seems that all
system dependencies that have only C code in them will work without being
rebuilt (though I have found the following exceptions, sometimes just for a
specific sub-architecture: gsl and tiff).

The netcdf stuff in spatial324.zip works just fine with ncdf4 and Rnetcdf,
I can build and check both of those packages under the new toolchain. I
really think the problem has to do with the fact that mzR comes with its
own netcdf library and header file which should not be necessary, and it
causes the duplicate symbol error we see. I think it just needs a tweak to
its Makevars.win but I haven't hit on the right one.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#36 (comment)

@dtenenba
Copy link
Contributor

Your understanding is correct, or at least it's what I think is going on. Don't hesitate to contact me if you have questions as you look into it. Thanks.

@dtenenba
Copy link
Contributor

At a glance I would say the Rnetcdf Makevars.win is more informative than the ncdf4 one.

@kasperdanielhansen
Copy link

well, yeah, turns out spatial324 includes both libnetcdf and libnetcdf4 and
Rnetcdf uses libnetcdf and ncdf4 uses libnetcdf4. So we need to figure out
which one is best for mzR.

Comments on this Steffen, Laurent?

On Wed, Apr 13, 2016 at 11:15 PM, dtenenba notifications@github.com wrote:

At a glance I would say the Rnetcdf Makevars.win
https://github.com/cran/RNetCDF/blob/master/src/Makevars.win is more
informative than the ncdf4 one.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#36 (comment)

@dtenenba
Copy link
Contributor

Based on the contents of the src/win/x64 directory, I'd say mzR has been using libnetcdf.

@kasperdanielhansen
Copy link

Dan, I have a build environment set up, I have replicated the mzR error and
I can install affxparser from source using the new GCC, so I should be set
to go. Unlikely I will get much work done tomorrow, but given the current
state I am pretty confident I can fix this over the weekend.

On Thu, Apr 14, 2016 at 12:01 AM, dtenenba notifications@github.com wrote:

Based on the contents of the src/win/x64
https://github.com/sneumann/mzR/tree/master/src/win/x64 directory, I'd
say mzR has been using libnetcdf.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#36 (comment)

@lgatto
Copy link
Collaborator

lgatto commented Apr 14, 2016

Based on the contents of the src/win/x64 directory, I'd say mzR has been using libnetcdf.

Yes, that seems to be the right one.

@jimhester
Copy link

Ok I can reproduce the error multiple definition errors as well if I setup LOCAL_SOFT as you describe. They can be fixed by removing the -D_MSC_VER define from Makevars.win

Index: src/Makevars.win
===================================================================
--- src/Makevars.win    (revision 116189)
+++ src/Makevars.win    (working copy)
@@ -7,7 +7,7 @@

 ## Use the R_HOME indirection to support installations of multiple R version
 PKG_CPPFLAGS=-D_LARGEFILE_SOURCE $(PWIZ_CPPFLAGS)  # last include only for cross-compiling
-PKG_CFLAGS=-D_LARGEFILE_SOURCE -I./win/$(R_ARCH) -D_MSC_VER  -fgnu89-inline
+PKG_CFLAGS=-D_LARGEFILE_SOURCE -I./win/$(R_ARCH) -fgnu89-inline

 ## Faster Linker on BioC build farm:
 ## https://github.com/sneumann/mzR/issues/21#issuecomment-87802019

Also if you are using netcdf from LOCAL_SOFT it can be removed completely from src/win/{x64,i386}/ without issue.

The vector allocation issue on 32bit remains open however.

@dtenenba
Copy link
Contributor

Thanks so much @jimhester -- it works! Unfortunately I am now getting the i386 issue on moscato2 whereas I wasn't before. I wonder if removing the -D_MSC_VER somehow triggers that. So now we need to figure that out. See above for the exact message.

Does anyone have any thoughts on that?
Again, 32-bit R on windows cannot use more than 3GB memory all told, and apparently loading the Ramp module either uses this much or puts the total use above 3GB. Not sure why this didn't happen before the new toolchain, but is there a quick fix?

@dtenenba
Copy link
Contributor

I was talking to @mtmorgan about this and he suggested that it might be useful for someone to:

  • Install mzR under i386 with the --no-test-load flag so that the install succeeds
  • Run gdb under 32-bit R so we can see where that long vector is being allocated.

I'm not sure I have the chops to debug C++ code like this, but someone else might want to.
There are some instructions for running gdb in R on windows that might be helpful.

The puzzling thing is that mzR has not changed, so why is it using more memory now than before? It could be a new toolchain issue, or (just guessing) it could be related to a relatively recent update of Rcpp on CRAN (March 26, but we have been blocked by the x64 issue for a while and might not have noticed the i386 issue before).

@dtenenba
Copy link
Contributor

Just noticed something interesting. The package BubbleTree is failing in the latest build report:

https://bioconductor.org/checkResults/devel/bioc-LATEST/BubbleTree/moscato2-checksrc.html

The error is:

Error in .doLoadActions(where, attach) : 
  error in load action .__A__.1 for package multicool: loadModule(module = "Multicool", what = TRUE, env = ns, loadNow = TRUE): Unable to load module "Multicool": cannot allocate vector of length 1790997466
ERROR: lazy loading failed for package 'BubbleTree'

Notice that the length of the vector it is trying to allocate is EXACTLY the same as what we see above when trying to install mzR for i386.
BubbleTree itself does not use Rcpp but something in its dependency stack loads multicool which uses Rcpp and loads Rcpp modules, just like mzR. So this could be a problem with Rcpp after all. I will try and find a minimal reproducible example and tell the Rcpp folks about it.

@dtenenba
Copy link
Contributor

What's interesting is that CRAN's build system does not seem to have this issue, (even though moscato2 and both Jim's and my VM's have it) because I can install the binary version of multicool and it loads fine under i386, but if I try and build it from source, it fails. Reported this to Rcpp:

RcppCore/Rcpp#462

@kasperdanielhansen
Copy link

One thing I'll try to address is that the way mzR uses Rcpp is not what is currently recommended, as far as I can see; I'm not an Rcpp expert. This is probably because Rcpp changes so fast and mzR has not been updated. Anyway, worth keeping in mind.

Best,
Kasper
(Sent from my phone.)

On Apr 14, 2016, at 15:17, dtenenba notifications@github.com wrote:

Just noticed something interesting. The package BubbleTree is failing in the latest build report:

https://bioconductor.org/checkResults/devel/bioc-LATEST/BubbleTree/moscato2-checksrc.html

The error is:

Error in .doLoadActions(where, attach) :
error in load action .A.1 for package multicool: loadModule(module = "Multicool", what = TRUE, env = ns, loadNow = TRUE): Unable to load module "Multicool": cannot allocate vector of length 1790997466
ERROR: lazy loading failed for package 'BubbleTree'
Notice that the length of the vector it is trying to allocate is EXACTLY the same as what we see above when trying to install mzR for i386.
BubbleTree itself does not use Rcpp but something in its dependency stack loads multicool which uses Rcpp and loads Rcpp modules, just like mzR. So this could be a problem with Rcpp after all. I will try and find a minimal reproducible example and tell the Rcpp folks about it.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@dtenenba
Copy link
Contributor

OK, so leaving aside the issue that we can't update to the latest Rcpp because it isn't available on CRAN, I tried building mzR under the latest Rcpp from github and got a different error:

** testing if installed package can be loaded
Found more than one class "Rcpp_Ramp" in cache; using the first, from namespace 'mzR'
Found more than one class "Rcpp_Ramp" in cache; using the first, from namespace 'mzR'
Error : .onLoad failed in loadNamespace() for 'mzR', details:
  call: value[[3L]](cond)
  error: failed to load module Ramp from package mzR
could not find function ".moduleMetaName"

Also, I'd like to try following Dirk's suggestion that mzR explicitly use C++11, not sure how to make that happen? Is it just a matter of adding C++11 to the SystemRequirements field in the DESCRIPTION file?

@dtenenba
Copy link
Contributor

OK, just checked into svn a version of mzR that installs under both architectures (I think, need to confirm tonight) if you are running the latest Rcpp from github. Of course we need this to be in CRAN to really help us, they say it will go in before the R.3.3 release but it would be nice to clean up our build report before that.

@mtmorgan What do you think of putting the latest Rcpp in our extra repos and modifying mzR/DESCRIPTION to require that version? Just as a temporary measure...

@kasperdanielhansen
Copy link

Thanks Dan,

In this case I think you need to put the Rcpp fix on the build servers. As
I understand this thread and the Rcpp issue Dan filed, basically Rcpp is
broken on Windows right now. And they don't intend to push a release until
right as R is released - probably because they are working on their own
problems. If we don't push Rcpp to the build servers there could be
windows problems we don't encounter until the last minute. And for most
people, fixing windows is painful.

Best,
Kasper

On Thu, Apr 14, 2016 at 6:18 PM, dtenenba notifications@github.com wrote:

OK, just checked into svn a version of mzR that installs under both
architectures (I think, need to confirm tonight) if you are running the
latest Rcpp from github. Of course we need this to be in CRAN to really
help us, they say it will go in before the R.3.3 release but it would be
nice to clean up our build report before that.

@mtmorgan https://github.com/mtmorgan What do you think of putting the
latest Rcpp in our extra repos and modifying mzR/DESCRIPTION to require
that version? Just as a temporary measure...


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#36 (comment)

@dtenenba
Copy link
Contributor

@kasperdanielhansen , I tend to agree in principle but Rcpp needs to pass check before I do that. We'd expect the same for any other CRAN or Bioc package. I am working with Rcpp folks over at RcppCore/Rcpp#463 and I think we're getting close.

@kasperdanielhansen
Copy link

I'm not going to argue with a requirement of passing R CMD check (well,
perhaps apart from man pages or something like that).

On Fri, Apr 15, 2016 at 12:41 PM, dtenenba notifications@github.com wrote:

@kasperdanielhansen https://github.com/kasperdanielhansen , I tend to
agree in principle but Rcpp needs to pass check before I do that. We'd
expect the same for any other CRAN or Bioc package. I am working with Rcpp
folks over at RcppCore/Rcpp#463
RcppCore/Rcpp#463 and I think we're getting
close.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#36 (comment)

@dtenenba
Copy link
Contributor

Rcpp now passes check! So I am in the process of putting it in our extra repos (for windows only) and verifying that mzR builds on both architectures. After that hopefully we can close this. Though mzR maintainers will need to be sure to merge recent changes from svn into github.

@dtenenba
Copy link
Contributor

Note that there will be no builds tonight but there will tomorrow night so hopefully we'll see a lot more green on Sunday.

@dtenenba
Copy link
Contributor

mzR now passes check on both architectures! Hopefully it and the packages that depend on it will be green on Sunday. Thanks everyone who helped out with this!

When the changes from svn are merged into master, this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants