[checkfornan] io.h for windows instead of unistd.h #117

proyan · 2021-08-28T08:09:53Z

Remaining fix from #112

CLAassistant · 2021-08-28T08:09:56Z

All committers have signed the CLA.

bradbell · 2021-08-28T09:23:24Z

I am not sure if you need !defined(__clang__) here ?

It seems <io.h> is required for MS to find mkstemp; see
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/mktemp?view=msvc-160
and
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/mktemp-wmktemp?view=msvc-160

Should we include this in the test for mkstemp in the cppad_has_mkstemp check in the file
https://github.com/coin-or/CppAD/blob/master/include/cppad/CMakeLists.txt
?

# -----------------------------------------------------------------------------
# cppad_has_mkstemp
#
SET(source "
# include <stdlib.h>
# include <unistd.h>
int main(void)
{
    char pattern[] = \"/tmp/fileXXXXXX\";
    int fd = mkstemp(pattern);
    return 0;
}
" )
compile_source_test("${source}" cppad_has_mkstemp )

proyan · 2021-08-28T09:39:24Z

I have updated the remaining instances of unistd.h by io.h in 5808656. I'm running make check in the CI to see if there are other errors that I may have missed.

proyan · 2021-08-28T09:40:44Z

It seems io.h is the replacement for unistd in windows systems https://stackoverflow.com/questions/341817/is-there-a-replacement-for-unistd-h-for-windows-visual-c. So normally this should not be compiler dependent. I don't think we need to add !defined(clang) here

bradbell · 2021-08-28T10:37:06Z

I am not sure if you need !defined(__clang__) here ?

It seems <io.h> is required for MS to find mkstemp; see
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/mktemp?view=msvc-160
and
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/mktemp-wmktemp?view=msvc-160

Should we include this in the test for mkstemp in the cppad_has_mkstemp check in the file
https://github.com/coin-or/CppAD/blob/master/include/cppad/CMakeLists.txt
?

# -----------------------------------------------------------------------------
# cppad_has_mkstemp
#
SET(source "
# include <stdlib.h>
# include <unistd.h>
int main(void)
{
    char pattern[] = \"/tmp/fileXXXXXX\";
    int fd = mkstemp(pattern);
    return 0;
}
" )
compile_source_test("${source}" cppad_has_mkstemp )

bradbell · 2021-08-28T10:40:04Z

I closed this bug by mistake and have re-opened it ?

I did some testing on windows and could not get it to find mkstemp. Then I realized that the link above for io.h was referring to mktemp which is not the same as mkstemp. It seems windows does not have a verision of he posix routine mkstemp ?
https://pubs.opengroup.org/onlinepubs/9699919799/functions/mkdtemp.html

bradbell · 2021-08-28T12:41:18Z

Does an mkstemp function exist when using your Windows version of the clang compiler ?

proyan · 2021-08-30T09:03:08Z

@bradbell I've added the ci with make check for windows and ubuntu with github actions. I think somehow github actions might be disabled on the main repo, I can see the CI run on my fork at https://github.com/proyan/CppAD/actions.

proyan · 2021-08-30T09:08:48Z

Does an mkstemp function exist when using your Windows version of the clang compiler ?

I see the following errors in the CI https://github.com/Simple-Robotics/pycppad/pull/18/checks?check_run_id=3459595658 :

lld-link : error : undefined symbol: omp_set_dynamic [D:\a\pycppad\pycppad\CppAD\build\example\multi_thread\openmp\example_multi_thread_openmp.vcxproj]
  >>> referenced by example_multi_thread_openmp.dir\Release\a11c_openmp.obj:(bool __cdecl a11c(void))
  >>> referenced by example_multi_thread_openmp.dir\Release\simple_ad_openmp.obj:(bool __cdecl simple_ad(void))
  >>> referenced by example_multi_thread_openmp.dir\Release\team_openmp.obj:(bool __cdecl team_create(unsigned __int64))
  
lld-link : error : undefined symbol: omp_set_num_threads [D:\a\pycppad\pycppad\CppAD\build\example\multi_thread\openmp\example_multi_thread_openmp.vcxproj]
  >>> referenced by example_multi_thread_openmp.dir\Release\a11c_openmp.obj:(bool __cdecl a11c(void))
  >>> referenced by example_multi_thread_openmp.dir\Release\simple_ad_openmp.obj:(bool __cdecl simple_ad(void))
  >>> referenced by example_multi_thread_openmp.dir\Release\simple_ad_openmp.obj:(bool __cdecl simple_ad(void))
  >>> referenced 2 more times
  
lld-link : error : undefined symbol: __kmpc_fork_call [D:\a\pycppad\pycppad\CppAD\build\example\multi_thread\openmp\example_multi_thread_openmp.vcxproj]
  >>> referenced by example_multi_thread_openmp.dir\Release\a11c_openmp.obj:(bool __cdecl a11c(void))
  >>> referenced by example_multi_thread_openmp.dir\Release\simple_ad_openmp.obj:(bool __cdecl simple_ad(void))
  >>> referenced by example_multi_thread_openmp.dir\Release\team_openmp.obj:(bool __cdecl team_work(void (__cdecl *)(void)))
  
lld-link : error : undefined symbol: __kmpc_for_static_init_4 [D:\a\pycppad\pycppad\CppAD\build\example\multi_thread\openmp\example_multi_thread_openmp.vcxproj]
  >>> referenced by example_multi_thread_openmp.dir\Release\a11c_openmp.obj:(.omp_outlined.)
  >>> referenced by example_multi_thread_openmp.dir\Release\simple_ad_openmp.obj:(.omp_outlined.)
  >>> referenced by example_multi_thread_openmp.dir\Release\team_openmp.obj:(.omp_outlined.)
  
lld-link : error : undefined symbol: __kmpc_for_static_fini [D:\a\pycppad\pycppad\CppAD\build\example\multi_thread\openmp\example_multi_thread_openmp.vcxproj]
  >>> referenced by example_multi_thread_openmp.dir\Release\a11c_openmp.obj:(.omp_outlined.)
  >>> referenced by example_multi_thread_openmp.dir\Release\simple_ad_openmp.obj:(.omp_outlined.)
  >>> referenced by example_multi_thread_openmp.dir\Release\team_openmp.obj:(.omp_outlined.)
  
lld-link : error : undefined symbol: omp_get_thread_num [D:\a\pycppad\pycppad\CppAD\build\example\multi_thread\openmp\example_multi_thread_openmp.vcxproj]
  >>> referenced by example_multi_thread_openmp.dir\Release\simple_ad_openmp.obj:(unsigned __int64 __cdecl `anonymous namespace'::thread_number(void))
  >>> referenced by example_multi_thread_openmp.dir\Release\team_openmp.obj:(bool __cdecl team_create(unsigned __int64))
  >>> referenced by example_multi_thread_openmp.dir\Release\team_openmp.obj:(unsigned __int64 __cdecl `anonymous namespace'::thread_num(void))
  >>> referenced 2 more times
  
lld-link : error : undefined symbol: omp_in_parallel [D:\a\pycppad\pycppad\CppAD\build\example\multi_thread\openmp\example_multi_thread_openmp.vcxproj]
  >>> referenced by example_multi_thread_openmp.dir\Release\simple_ad_openmp.obj:(bool __cdecl `anonymous namespace'::in_parallel(void))
  >>> referenced by example_multi_thread_openmp.dir\Release\team_openmp.obj:(bool __cdecl team_create(unsigned __int64))
  >>> referenced by example_multi_thread_openmp.dir\Release\team_openmp.obj:(bool __cdecl `anonymous namespace'::in_parallel(void))
  >>> referenced 2 more times
  Building Custom Rule D:/a/pycppad/pycppad/CppAD/introduction/CMakeLists.txt
  introduction.vcxproj -> D:\a\pycppad\pycppad\CppAD\build\introduction\Release\introduction.exe
  Begin test group introduction

however, it still says that all tests are passing

jcarpent · 2021-08-30T09:15:37Z

You just need to link against openmp. Depending on the OS/compiler, this might not be needed.

bradbell · 2021-08-30T11:31:04Z

What do you get when you do the following:
cmake your_command_line_arguments | grep multi_thread

proyan · 2021-08-31T08:37:43Z

What do you get when you do the following:
cmake your_command_line_arguments | grep multi_thread

On my Ubuntu system, I get

-- make check_example_multi_thread_openmp: available
-- make check_example_multi_thread_pthread: available
-- boost_multi_thread_ok = 1
-- make check_example_multi_thread_bthread: available
-- make check_example_multi_thread: available

I will try on the windows CI. I think the suggestion of justin to link the examples with openmp should be sufficient

bradbell · 2021-08-31T11:52:26Z

@proyan Try the following:

make check_example_multi_thread_openmp

proyan · 2021-08-31T12:34:54Z

Regarding openmp linking, it seems that with visual studio and clang combination, this is a known issue https://stackoverflow.com/questions/62122247/openmp-linking-errors-in-visual-studio-2019-llvm

proyan · 2021-08-31T12:36:34Z

@proyan Try the following:
make check_example_multi_thread_openmp

In windows-clang, I get the undefined symbol error from above. On ubuntu it passes

bradbell · 2021-08-31T12:39:52Z

@proyan It seems to me that there are three different things going on in this pull request:

Automated testing to determine if mkstemp is available on a windows system
Setting up a standard file structure and adding to the yml files for driving automated tests.
see the existing *.yml files in the CppAD master
https://github.com/coin-or/CppAD/blob/master/.travis.yml
https://github.com/coin-or/CppAD/blob/master/appveyor.yml
Fixing the linkage to omp library on window

Perhaps it would be better if we spit it up ?

proyan · 2021-08-31T12:47:29Z

I'm okay with reducing the scope of this PR.
I think eventually this PR does two things:

Add CI with github actions for windows, ubuntu and MacOS.
Fix issues with make check on windows. (which for now is only with clang compiler linking with openmp)

Regarding mkstemp, I didn't see the error related to mkstemp in the CI run https://github.com/proyan/CppAD/actions

bradbell · 2021-08-31T12:47:59Z

@proyan Try the following:
make check_example_multi_thread_openmp
In windows-clang, I get the undefined symbol error from above. On ubuntu it passes

I did not realize that make check does not run the available muti_thread checks. I will need to look into why this is the case.

bradbell · 2021-08-31T14:01:07Z

What is the purpose of the file .github/workflows/ci-windows-clang.yml .
It seems to be installing CppAD for use by pycppad ?

bradbell · 2021-08-31T14:06:45Z

I fixed make check so it includes the multi_thread examples; see
3667ead

bradbell · 2021-08-31T14:31:24Z

Looking at
bc40d0f

It seems the omp library may be system specific and that perhaps it would be better to use OpenMP_CXX_LIB_NAMES; see
https://cmake.org/cmake/help/latest/module/FindOpenMP.html
?

…ps://devblogs.microsoft.com/cppblog/improved-openmp-support-for-cpp-in-visual-studio/

proyan · 2021-08-31T14:48:51Z

What is the purpose of the file .github/workflows/ci-windows-clang.yml .
It seems to be installing CppAD for use by pycppad ?

The github workflows files are only adding a CI (as seen here: https://github.com/proyan/CppAD/actions) that performs make check on pushes and pull requests. So this is independent from any other repository. I'm not sure why these are not being shown in this PR as well, I think the github actions might be disabled in the github repo.

proyan · 2021-08-31T15:05:49Z

It seems the omp library may be system specific and that perhaps it would be better to use OpenMP_CXX_LIB_NAMES; see
https://cmake.org/cmake/help/latest/module/FindOpenMP.html

You're right, it wasn't correct to add the library linkage like I did. However, I think the CXX_FLAGS are already adding the linking specifications for openmp, so the issue isn't whether openmp can be found, but how to link for the windows+clang case. Could you please have a look at ddc1802?

bradbell · 2021-08-31T15:24:20Z

It seems to me that the variable

OpenMP_<lang>_FLAGS
    OpenMP compiler flags for <lang>, separated by spaces.

with = CXX should have all the necessary flags for the system ?

proyan · 2021-08-31T15:35:07Z

I don't think OpenMP CXX Flags are being correctly fixed based on the compiler/environment types.
From the latest commit , I still get the error in the following CI. So the fix didn't work:
https://github.com/proyan/CppAD/runs/3474261730?check_suite_focus=true

2021-08-31T15:15:22.6299194Z lld-link : warning : ignoring unknown argument '-Xclang' [D:\a\CppAD\CppAD\build\test_more\cppad_for_tmb\test_more_cppad_for_tmb.vcxproj]
2021-08-31T15:15:22.6312512Z lld-link : warning : ignoring unknown argument '-fopenmp' [D:\a\CppAD\CppAD\build\test_more\cppad_for_tmb\test_more_cppad_for_tmb.vcxproj]
2021-08-31T15:15:22.6516716Z lld-link : error : undefined symbol: omp_set_dynamic [D:\a\CppAD\CppAD\build\test_more\cppad_for_tmb\test_more_cppad_for_tmb.vcxproj]
2021-08-31T15:15:22.6519205Z   >>> referenced by test_more_cppad_for_tmb.dir\Release\multi_atomic_two.obj:(bool __cdecl multi_atomic_two(void))
2021-08-31T15:15:22.6528655Z   >>> referenced by test_more_cppad_for_tmb.dir\Release\multi_atomic_three.obj:(bool __cdecl multi_atomic_three(void))
2021-08-31T15:15:22.6530062Z   >>> referenced by test_more_cppad_for_tmb.dir\Release\multi_chkpoint_one.obj:(bool __cdecl multi_chkpoint_one(void))
2021-08-31T15:15:22.6533783Z   >>> referenced 1 more times

I'm out of ideas for how to proceed here.

… ref:https://devblogs.microsoft.com/cppblog/improved-openmp-support-for-cpp-in-visual-studio/" This reverts commit ddc1802.

bradbell · 2021-09-03T20:06:56Z

I have started a branch for testing ideas related to github actions; see
https://github.com/coin-or/CppAD/tree/actions

bradbell · 2021-09-04T15:18:49Z

Using github actions on macos-latest,(on the actions branch) I get the following message

/Users/runner/work/CppAD/CppAD/include/cppad/core/check_for_nan.hpp:211:21: warning: 'tmpnam' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of tmpnam(3), it is highly recommended that you use mkstemp(3) instead. [-Wdeprecated-declarations]
        file_name = tmpnam( nullptr );

This indicates that macos is not finding mkstemp.

bradbell · 2021-09-06T14:07:22Z

I have done a lot of testing and cannot figure out why cmake on the macos-latest platform is not finding mkstemp.

Here is a a very similar test script that works in another repository:
https://github.com/bradbell/tst_github/blob/main/mkstemp/CMakeLists.txt
Here is the resulting test output
https://github.com/bradbell/tst_github/runs/3525307701?check_suite_focus=true
You will note that this script finds mkstemp because cmake has the following output

-- cxx_has_mkstemp_1 = 1
-- cxx_has_mkstemp_2 = 1

On the other hand, for the cppad version of the test (which I expect to be the same), has the following cmake output

 -- cppad_has_mkstemp = 0

TheCppAD actions branch has a version of the test that comments out the make check (so it runs much faster):
https://github.com/coin-or/CppAD/blob/actions/.github/workflows/ci_test.yml
The corresponding cmake output can be seen here
https://github.com/coin-or/CppAD/runs/3525397477?check_suite_focus=true
where you will see

 -- cppad_has_mkstemp = 0

bradbell · 2021-12-10T13:01:51Z

@proyan
Sorry for the delay here. I was very busy with some things for work and forgot about this pull request.
If you ware willing to maintain the files in the workflows directory, I will merge them into the coin-or CppAD repository.

proyan · 2021-12-11T00:03:15Z

Hi Brad,
Sure. Maintaining the github workflow files should not be too taxing. You could go ahead and merge the PR from my side

bradbell · 2021-12-11T11:15:04Z

config.guess is not a real source file. It is automatically created by the autotools install (which I want to get rid of but coin-or is using). see
https://coin-or.github.io/CppAD/doc/autotools.htm

Would you please merge the current master into your pull and test it so that I can fast forward the merge.

When I try to do so, I get an error:

git checkout master
git pull --ff-only
git fetch origin pull/117/head:workflow
git checkout workflow
git merge master --ff-only

fatal: Not possible to fast-forward, aborting

bradbell · 2022-01-24T22:46:05Z

@proyan Travis was sold and it appears they are no longer building the CppAD travis file
https://github.com/coin-or/CppAD/blob/master/.travis.yml

I plan to use github workflows to do a lot more CppAD testing and would like your help in this regard.

One big problem I have right now is testing on a ppc64le system; see the build.log for
https://koji.fedoraproject.org/koji/taskinfo?taskID=81757910
where the following text appears

to_string           Error
vectorBool          OK
memory_leak         OK
1 tests failed.
End test group example/utility
make[3]: *** [example/utility/CMakeFiles/check_example_utility.dir/build.make:70: example/utility/CMakeFiles/check_example_utility] Error 1

Do you know how I could get github actions to reproduce this problem ?

bradbell · 2022-01-24T23:07:45Z

@proyan A friend suggested trying
https://github.com/uraimo/run-on-arch-action
I will make an attempt and see what happens.

[checkfornan] io.h for windows instead of unistd.h

b4ffb0e

proyan mentioned this pull request Aug 28, 2021

Windows Support in CppAD #112

Closed

[unistd][io] replace unistd by io for windows.

5808656

bradbell closed this Aug 28, 2021

bradbell reopened this Aug 28, 2021

proyan added 4 commits August 30, 2021 10:55

[ci][conda] add conda env

a22d49b

[ci-windows-clang] add ci with make check

00260f3

[ci][ubuntu] add ubuntu ci

6e5e3c2

[ci][windows] ci-windows v142

cc00015

[CMakeLists][windows] link openmp to examples

bc40d0f

[multithread][openmp] openmp with clang requires -openmp:llvm ref:htt…

ddc1802

…ps://devblogs.microsoft.com/cppblog/improved-openmp-support-for-cpp-in-visual-studio/

[ci] fix environment name

c78eeff

proyan force-pushed the master branch from 93d82dd to cf61128 Compare September 2, 2021 08:19

Revert "[multithread][openmp] openmp with clang requires -openmp:llvm…

803ae08

… ref:https://devblogs.microsoft.com/cppblog/improved-openmp-support-for-cpp-in-visual-studio/" This reverts commit ddc1802.

proyan force-pushed the master branch from cf61128 to 803ae08 Compare September 2, 2021 08:23

This was referenced May 18, 2022

Fix Windows issues + add CI #143

Closed

Extend the CI for Windows/Ubuntu/Mac using conda #145

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[checkfornan] io.h for windows instead of unistd.h #117

[checkfornan] io.h for windows instead of unistd.h #117

proyan commented Aug 28, 2021

CLAassistant commented Aug 28, 2021 •

edited

bradbell commented Aug 28, 2021

proyan commented Aug 28, 2021

proyan commented Aug 28, 2021 •

edited

bradbell commented Aug 28, 2021

bradbell commented Aug 28, 2021 •

edited

bradbell commented Aug 28, 2021

proyan commented Aug 30, 2021

proyan commented Aug 30, 2021 •

edited

jcarpent commented Aug 30, 2021

bradbell commented Aug 30, 2021

proyan commented Aug 31, 2021

bradbell commented Aug 31, 2021

proyan commented Aug 31, 2021

proyan commented Aug 31, 2021

bradbell commented Aug 31, 2021

proyan commented Aug 31, 2021

bradbell commented Aug 31, 2021

bradbell commented Aug 31, 2021

bradbell commented Aug 31, 2021

bradbell commented Aug 31, 2021

proyan commented Aug 31, 2021

proyan commented Aug 31, 2021

bradbell commented Aug 31, 2021

proyan commented Aug 31, 2021

bradbell commented Sep 3, 2021

bradbell commented Sep 4, 2021

bradbell commented Sep 6, 2021

bradbell commented Dec 10, 2021

proyan commented Dec 11, 2021

bradbell commented Dec 11, 2021 •

edited

bradbell commented Jan 24, 2022

bradbell commented Jan 24, 2022 •

edited

[checkfornan] io.h for windows instead of unistd.h #117

Are you sure you want to change the base?

[checkfornan] io.h for windows instead of unistd.h #117

Conversation

proyan commented Aug 28, 2021

CLAassistant commented Aug 28, 2021 • edited

bradbell commented Aug 28, 2021

proyan commented Aug 28, 2021

proyan commented Aug 28, 2021 • edited

bradbell commented Aug 28, 2021

bradbell commented Aug 28, 2021 • edited

bradbell commented Aug 28, 2021

proyan commented Aug 30, 2021

proyan commented Aug 30, 2021 • edited

jcarpent commented Aug 30, 2021

bradbell commented Aug 30, 2021

proyan commented Aug 31, 2021

bradbell commented Aug 31, 2021

proyan commented Aug 31, 2021

proyan commented Aug 31, 2021

bradbell commented Aug 31, 2021

proyan commented Aug 31, 2021

bradbell commented Aug 31, 2021

bradbell commented Aug 31, 2021

bradbell commented Aug 31, 2021

bradbell commented Aug 31, 2021

proyan commented Aug 31, 2021

proyan commented Aug 31, 2021

bradbell commented Aug 31, 2021

proyan commented Aug 31, 2021

bradbell commented Sep 3, 2021

bradbell commented Sep 4, 2021

bradbell commented Sep 6, 2021

bradbell commented Dec 10, 2021

proyan commented Dec 11, 2021

bradbell commented Dec 11, 2021 • edited

bradbell commented Jan 24, 2022

bradbell commented Jan 24, 2022 • edited

CLAassistant commented Aug 28, 2021 •

edited

proyan commented Aug 28, 2021 •

edited

bradbell commented Aug 28, 2021 •

edited

proyan commented Aug 30, 2021 •

edited

bradbell commented Dec 11, 2021 •

edited

bradbell commented Jan 24, 2022 •

edited