Try win #48

ocefpaf · 2017-11-09T18:33:19Z

@pedro-vicente and @czender I am putting my branch as a PR here to make it more visible.

I'll merge #46 and #47 first, then re-base this one, and then it will look "cleaner," with only the Windows stuff.

conda-forge-linter · 2017-11-09T18:33:23Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

ocefpaf · 2017-11-10T11:51:26Z

The Windows builds are completing but an OPeNDAP test is not passing:

(C:\bld\nco_1510274210567\_t_env) (root) C:\bld\nco_1510274210567\test_tmp>if errorlevel 1 exit 1 
(C:\bld\nco_1510274210567\_t_env) (root) C:\bld\nco_1510274210567\test_tmp>ncks -M "http://tds.marine.rutgers.edu/thredds/dodsC/roms/espresso/2013_da/his/ESPRESSO_Real-Time_v2_History_Best" 
(C:\bld\nco_1510274210567\_t_env) (root) C:\bld\nco_1510274210567\test_tmp>if errorlevel 1 exit 1

Maybe nco is having issues with conda-forge's curl? Not sure what is going on. There are no error messages on AppVeyor and when I try it locally on a Windows machine I get a crash and an odd error message.

ocefpaf · 2017-11-22T12:20:35Z

recipe/bld.bat

+      -D EXPAT_LIBRARY=%LIBRARY_LIB%\expat.lib ^
+      -D CURL_LIBRARY=%LIBRARY_LIB%\libcurl.lib ^
+      -D LIB_ANTLR=%LIBRARY_LIB%\antlr.lib ^
+      -D HEADER_ANTLR=%LIBRARY_INC% ^


@pedro-vicente is this correct? I got this warning on Appveyor:

CMake Warning: Manually-specified variables were not used by the project: GSL_CBLAS_LIBRARY GSL_INCLUDE GSL_LIBRARY HEADER_ANTLR LIB_ANTLR ZLIB_LIBRARY

later on the library is found but the headers are now, see https://ci.appveyor.com/project/conda-forge/nco-feedstock/build/1.0.24#L693

The conda package for antlr is here and the headers are in the Library\include directory. Note that I copied them manually b/c the build process does not place them there, so maybe I missed some?

If you compare the entry in CMakeLists.txt

find_path(ANTLR_INCLUDE antlr/AST.hpp ${find_opt})

with the example in /cmake/build.bat

-DANTLR_INCLUDE:PATH=I:/antlr-2.7.7/lib/cpp

in this case, it means that this file must exist

I:/antlr-2.7.7/lib/cpp/antlr/AST.hpp

On my local Anaconda build this file is in

C:\Users\pedro\Anaconda3\conda-bld\nco_1511748388916\_h_env\Library\include

and this folder seems to contain lots of different files from different libraries...
Not sure why this is done this way but it does not look like a good way to place the files.
A better way would be to separate them by libraries

I sent a PR to antlr-feedstock to put the headers in a antlr subfolder. Let's see how that goes.
(Thanks @isuruf for the tip!)

ocefpaf · 2017-12-13T15:42:30Z

The Windows antlr package is still not OK:

I can confirm that the files are missing. I'll try to fix that over the weekend.

fatal error C1083: Cannot open include file: 'antlr/AST.hpp': No such file or directory

bollwyvl · 2017-12-13T17:24:59Z

Good catch!

ocefpaf · 2017-12-13T19:06:58Z

The antlr issue was solved by conda-forge/antlr-feedstock#17
Now we have a new one with gsl:

'C:\bld\nco_1513191229299\work\build\vc140.pdb'; linking object as if no debug info
LINK : warning LNK4098: defaultlib 'LIBCMT' conflicts with use of other libs; use /NODEFAULTLIB:library
fmc_gsl_cls.cc.obj : error LNK2001: unresolved external symbol gsl_rng_default
fmc_gsl_cls.cc.obj : error LNK2001: unresolved external symbol gsl_interp_linear
fmc_gsl_cls.cc.obj : error LNK2001: unresolved external symbol gsl_interp_polynomial
fmc_gsl_cls.cc.obj : error LNK2001: unresolved external symbol gsl_interp_cspline
fmc_gsl_cls.cc.obj : error LNK2001: unresolved external symbol gsl_interp_cspline_periodic
fmc_gsl_cls.cc.obj : error LNK2001: unresolved external symbol gsl_interp_akima
fmc_gsl_cls.cc.obj : error LNK2001: unresolved external symbol gsl_interp_akima_periodic
ncap2.exe : fatal error LNK1120: 7 unresolved externals
NMAKE : fatal error U1077: 'C:\bld\nco_1513191229299\_b_env\Library\bin\cmake.exe' : return code '0xffffffff'
Stop.
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64\nmake.exe"' : return code '0x2'
Stop.
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64\nmake.exe"' : return code '0x2'
Stop.
(C:\bld\nco_1513191229299\_b_env) C:\bld\nco_1513191229299\work\build>if errorlevel 1 exit 1

I am trying to compile without gsl now.

ocefpaf · 2017-12-13T19:11:09Z

OK, now tons of errors with antlr. See https://ci.appveyor.com/project/ocefpaf/nco-feedstock/build/1.0.39

Trying without antlr 😒

ocefpaf · 2017-12-13T19:24:51Z

@pedro-vicente without antlr and gsl we have success using latest master. See https://ci.appveyor.com/project/ocefpaf/nco-feedstock/build/1.0.41

pedro-vicente · 2017-12-13T22:32:36Z

@ocefpaf
The gsl error. Cmake prints this

-- Found GSL library at: C:/Users/pedro/Anaconda3/conda-bld/nco_1513203198504/_h_env/Library/lib/gsl.lib
-- Found GSL CBLAS library at: C:/Users/pedro/Anaconda3/conda-bld/nco_1513203198504/_h_env/Library/lib/gslcblas.lib

But the message"Found GSL" is a bit misleading. It just means that you added the option

-DGSL_LIBRARY:FILE=I:/gsl-1.8/src/gsl/1.8/gsl-1.8/VC8/libgsl/Debug-StaticLib/libgsl_d.lib

but you have to make sure the files "gslcblas.lib" or "libgsl_d.lib" exist.

In your run , they do not exist, so, you get the linking error.

I'll try to modify the Cmake script so that it confirms that the files really exist, so that it fails sooner and you know why.

regarding the ANTLR error it might be because you are using the library built by the project I sent, that might be different from the other projects in some settings. Are the other Visual Studio projects generated by Cmake?
I'll try to make a CMake script for ANTLR, and that error should go away this way.

czender · 2017-12-13T22:39:51Z

Very encouraging overall. The Antlr errors are due to linking issues, not compiler/syntax issues. Since Antlr is C++ the issues could be due to altered compiler settings used to build Antlr vs ncap2. Thanks for your efforts Filipe and Pedro!

isuruf · 2017-12-14T02:50:52Z

@ocefpaf,
Antlr was configured to use /MD while this project was using /MTd. Almost all MSVC libraries in conda-forge are built with /MD, so it makes sense to switch to /MD

Add this patch, isuruf/nco@f7274d5 and then add -DCMAKE_BUILD_TYPE=Release to build Release library (Remove d from /MTd) and add -DNCO_MSVC_USE_MT=no to switch from MT to MD.

ocefpaf · 2017-12-14T14:03:24Z

Antlr was configured to use /MD while this project was using /MTd. Almost all MSVC libraries in conda-forge are built with /MD, so it makes sense to switch to /MD

Thanks @isuruf! I have zero knowledge of how things work on Windows, I really appreciate your help!!

Add this patch, isuruf/nco@f7274d5 and then add -DCMAKE_BUILD_TYPE=Release to build Release library (Remove d from /MTd) and add -DNCO_MSVC_USE_MT=no to switch from MT to MD.

Done with the patch. Addressing the rest now. Thanks again!

isuruf · 2017-12-14T14:08:38Z

@ocefpaf, can you try the update patch isuruf/nco@f7274d5?

ocefpaf · 2017-12-14T14:12:21Z

@ocefpaf, can you try the update patch isuruf/nco@f7274d5?

Will do. Thanks!

then add -DCMAKE_BUILD_TYPE=Release to build Release library (Remove d from /MTd) and add -DNCO_MSVC_USE_MT=no to switch from MT to MD.

BTW, I am not sure where I need to "remove d from /MTd".

isuruf · 2017-12-14T14:13:22Z

BTW, I am not sure where I need to "remove d from /MTd".

Sorry, I meant adding -DCMAKE_BUILD_TYPE=Release would remove that part.

isuruf · 2017-12-14T15:03:27Z

Another patch on top of the last isuruf/nco@35cac9f

isuruf · 2017-12-14T15:41:49Z

Looks like antlr is compiled with /MT and HDF5 is built with /MD. How about switching antlr to /MD as well?

ocefpaf · 2017-12-14T15:45:17Z

Looks like antlr is compiled with /MT and HDF5 is built with /MD. How about switching antlr to /MD as well?

@isuruf nco is using antlr from the antlr2 branch and not the latest antlr 4. It is built using antlr.vcxproj and calling

msbuild.exe /p:Platform=%PLATFORM% /p:Configuration=Release lib\cpp\antlr.vcxproj

not sure what I need to change to reflect /MD vs /MT there.

isuruf · 2017-12-14T15:48:30Z

not sure what I need to change to reflect /MD vs /MT there.

Change

<RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary> to <RuntimeLibrary>MultiThreadedDebugDLL</RuntimeLibrary> and

<RuntimeLibrary>MultiThreaded</RuntimeLibrary> to <RuntimeLibrary>MultiThreadedDLL</RuntimeLibrary> in the vcxproj file

isuruf · 2017-12-14T15:57:01Z

recipe/bld.bat

+cmake -G "NMake Makefiles" ^
+      -D CMAKE_INSTALL_PREFIX=%LIBRARY_PREFIX% ^
+      -D CMAKE_BUILD_TYPE=Release ^
+      -D NCO_MSVC_USE_MT=no ^


Can you change this to yes? (Then there's no need for changing antlr)

To be honest I don't know. I'll try this first and, if does not work, I'll change antlr.

ocefpaf · 2017-12-14T16:08:53Z

@isuruf success! https://ci.appveyor.com/project/ocefpaf/nco-feedstock/build/1.0.46

However, I am inclined to try modifying antlr to try to keep everything consistent with /MD. Is that worth it?

ocefpaf · 2017-12-14T16:23:40Z

recipe/meta.yaml

@@ -42,7 +42,7 @@ requirements:
  run:
    - curl >=7.44.0,<8
    - expat 2.2.*
-    - gsl >=2.2,<2.3  # [not win]
+    - gsl >=2.2,<2.3


Using gsl on Windows is still a no-no. See https://ci.appveyor.com/project/ocefpaf/nco-feedstock/build/1.0.47

conda-forge/gsl-feedstock#15

isuruf · 2017-12-14T16:59:45Z

This says "All modules passed to a given invocation of the linker must have been compiled with the same run-time library compiler option (/MD, /MT, /LD).".

isuruf · 2017-12-15T08:02:41Z

recipe/meta.yaml

+  patches:
+    # See https://github.com/conda-forge/nco-feedstock/pull/48#issuecomment-351592929
+    - cmake.patch
+    - strcasecmp.patch


These two patches are in git master now

I updated to reflect that. Thanks!

@czender if this passes it would be nice to have a new release so we can make this "official."

I merged the patches upstream. @ocefpaf does everything work with Windows? Let me know. It would be nice, though not a deal-breaker, to have GSL functionality in the Windows version. Once this is done, or progress stalls, I'll happily tag and release NCO 4.7.1, which has no other major features pending. That should be the official beginning of availability on Windows conda-forge.

I merged the patches upstream.

Saw that. Thanks!

@ocefpaf does everything work with Windows?

We are testing the commands

ncks -H --trd -v one in.nc ncks --help ncks -M "http://tds.marine.rutgers.edu/thredds/dodsC/roms/espresso/2013_da/his/ESPRESSO_Real-Time_v2_History_Best"

and those are working fine. I'll add a simple ncap2 test, like for Linux. Do you suggest anything else?

BTW, we can set Appveyor up in the nco repo that would build this recipe and run the test. That would help catch bugs on Windows early.

Please do set up Appveyor, if you have time.
I don't particularly expect the Bash shell scripts ncclimo and ncremap to work with Windows, and I have no way to test that, so let's not worry about getting them working until after 4.7.1 is released.
I'm unclear on the status of GSL in the Windows build. Seems like there was a link problem. This would be a good test for any platform on which GSL is thought to work:
`zender@aerosol:~$ ncap2 -O -v -s 'erf_one=float(gsl_sf_erf(1.0f));print(erf_one,"%g")' ~/nco/data/in.nc ~/foo.nc

0.842701`

ocefpaf · 2017-12-15T21:43:36Z

recipe/meta.yaml

  commands:
    - ncks --help
+    - ncks -M "http://tds.marine.rutgers.edu/thredds/dodsC/roms/espresso/2013_da/his/ESPRESSO_Real-Time_v2_History_Best"
    - ncap2 --help


@czender a simple ncap2 call is working on Windows. See https://ci.appveyor.com/project/ocefpaf/nco-feedstock/build/1.0.55#L2008

isuruf · 2017-12-15T22:28:00Z

recipe/bld.bat

+cd %SRC_DIR%\build
+
+set "CFLAGS=%CFLAGS% -DWIN32 -DGSL_DLL"
+set "CXXFLAGS=%CXXFLAGS% -DWIN32 -DGSL_DLL"


@czender GSL error was fixed with this

Yep. The linking issue was fixed by @isuruf.

Also, I just added the extra test in my latest commit. See 391c080

ocefpaf · 2017-12-15T22:43:50Z

recipe/run_test.bat

+
+ncgen -o in.nc in.cdl
+
+ncap2 -O -v -s 'erf_one=float(gsl_sf_erf(1.0f));print(erf_one,"%g")' in.nc foo.nc


@czender it did not crash, but I have no idea how to make .bat files to print the value. I did test locally though and I got 0.842701. See https://ci.appveyor.com/project/ocefpaf/nco-feedstock/build/1.0.56#L1617

I guess we are all set! With a new release I'll update this and merge.

🎉 conda nco on windows 🎉

ocefpaf · 2017-12-16T12:12:06Z

@czender I'm merging this b/c I found a group of volunteer that want to test it out. We can replace the binaries later with the official release.

Many thanks to all who contribute to this (@isuruf, @rsignell-usgs, @gillins, @pedro-vicente, @czender, and many more testing it locally and sending a feedback.)

Having the conda nco on Windows is a big deal!

czender · 2017-12-16T16:13:12Z

I concur it's a BFD for NCO. Probably will release 4.7.1 late next week. Please stress it everyone and report any fixable issues. I'm grateful for everyone's help. Shout-outs to @rsignell-usgs for getting us on Conda initially, @pedro-vicente for the Windows + CMake support, @ocefpaf for the integration, and the helpful people I had never worked with before @isuruf @gillins and others who stepped-up for key patches (Antlr, GSL, linking) in the last few weeks. I will name 4.7.1 "Ajudar". Filipe and Pedro will know this is Portuguese with connotations of help, aid, and befriend.

ocefpaf mentioned this pull request Nov 10, 2017

Windows build #6

Closed

ocefpaf force-pushed the try_win branch 3 times, most recently from a005b8a to c8177b4 Compare November 13, 2017 15:12

ocefpaf force-pushed the try_win branch from 7d4712f to d6d1cb5 Compare November 22, 2017 10:37

ocefpaf commented Nov 22, 2017

View reviewed changes

ocefpaf force-pushed the try_win branch from d6d1cb5 to 394fd87 Compare November 27, 2017 13:11

rerender

8fe8afa

ocefpaf force-pushed the try_win branch from b152cdf to d7f052e Compare December 13, 2017 13:16

conda-forge deleted a comment from conda-forge-linter Dec 13, 2017

ocefpaf mentioned this pull request Dec 13, 2017

Fix Windows copy conda-forge/antlr-feedstock#17

Merged

ocefpaf added a commit to ocefpaf/nco-feedstock that referenced this pull request Dec 14, 2017

second part of conda-forge#48 (comment)

f2d5a44

isuruf reviewed Dec 14, 2017

View reviewed changes

ocefpaf mentioned this pull request Dec 14, 2017

try MD vs MT conda-forge/antlr-feedstock#18

Merged

ocefpaf commented Dec 14, 2017

View reviewed changes

isuruf reviewed Dec 15, 2017

View reviewed changes

ocefpaf force-pushed the try_win branch from b8c5fd9 to 9ef8d11 Compare December 15, 2017 11:45

Build nco on Windows.

7ceb10d

ocefpaf force-pushed the try_win branch from 9ef8d11 to 7ceb10d Compare December 15, 2017 11:48

call ncap2 cli help as a test

4ec7a62

ocefpaf commented Dec 15, 2017

View reviewed changes

isuruf reviewed Dec 15, 2017

View reviewed changes

ncap2 test

391c080

ocefpaf commented Dec 15, 2017

View reviewed changes

ocefpaf merged commit 27c1e8d into conda-forge:master Dec 16, 2017

ocefpaf deleted the try_win branch December 16, 2017 12:12

ocefpaf mentioned this pull request Dec 16, 2017

remove patches and add exit status #55

Merged


		ncgen -o in.nc in.cdl

		ncap2 -O -v -s 'erf_one=float(gsl_sf_erf(1.0f));print(erf_one,"%g")' in.nc foo.nc

Try win #48

Try win #48

Conversation

ocefpaf commented Nov 9, 2017

conda-forge-linter commented Nov 9, 2017

ocefpaf commented Nov 10, 2017

Choose a reason for hiding this comment

pedro-vicente Nov 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocefpaf commented Dec 13, 2017

bollwyvl commented Dec 13, 2017

ocefpaf commented Dec 13, 2017

ocefpaf commented Dec 13, 2017

ocefpaf commented Dec 13, 2017

pedro-vicente commented Dec 13, 2017

czender commented Dec 13, 2017

isuruf commented Dec 14, 2017 • edited Loading

ocefpaf commented Dec 14, 2017

isuruf commented Dec 14, 2017

ocefpaf commented Dec 14, 2017

isuruf commented Dec 14, 2017

isuruf commented Dec 14, 2017

isuruf commented Dec 14, 2017

ocefpaf commented Dec 14, 2017

isuruf commented Dec 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocefpaf commented Dec 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isuruf commented Dec 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocefpaf commented Dec 16, 2017

czender commented Dec 16, 2017

pedro-vicente Nov 27, 2017 •

edited

Loading

isuruf commented Dec 14, 2017 •

edited

Loading