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

[DRAFT] Switch to use autotools upstream build system also on Windows instead of vendored CMake and link ampl-asl also on Windows #123

Conversation

traversaro
Copy link
Contributor

@traversaro traversaro commented Nov 23, 2024

*DO NOT MERGE EVEN IF GREEN, AS PROBABLY THIS MAY CHANGE THE ABI, in case this works we will wait for a new release before merging.

Fix #55 .

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@traversaro
Copy link
Contributor Author

@conda-forge-admin, please rerender

@conda-forge-admin
Copy link
Contributor

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/meta.yaml) and found it was in an excellent condition.

conda-forge-webservices[bot] and others added 3 commits November 23, 2024 18:06
@traversaro
Copy link
Contributor Author

Not bad for the first run, something compiles but then there is this failure:

2024-11-23T18:29:04.0249502Z lld-link: error: undefined symbol: dmumps_c
2024-11-23T18:29:04.0251688Z >>> referenced by Algorithm/LinearSolvers/.libs/IpMumpsSolverInterface.o:(public: __cdecl Ipopt::MumpsSolverInterface::MumpsSolverInterface(void))
2024-11-23T18:29:04.0253191Z >>> referenced by Algorithm/LinearSolvers/.libs/IpMumpsSolverInterface.o:(public: virtual __cdecl Ipopt::MumpsSolverInterface::~MumpsSolverInterface(void))
2024-11-23T18:29:04.0254253Z >>> referenced by Algorithm/LinearSolvers/.libs/IpMumpsSolverInterface.o:(private: enum Ipopt::ESymSolverStatus __cdecl Ipopt::MumpsSolverInterface::SymbolicFactorization(void))
2024-11-23T18:29:04.0255051Z >>> referenced 5 more times
2024-11-23T18:29:04.0372308Z clang++: error: linker command failed with exit code 1 (use -v to see invocation)
2024-11-23T18:29:06.1985153Z ../../../../src/Apps/AmplSolver/AmplTNLP.cpp:18:10: fatal error: 'asl.h' file not found

@traversaro
Copy link
Contributor Author

@conda-forge-admin, please rerender

@conda-forge-admin
Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/11989503242. Examine the logs at this URL for more detail.

@traversaro
Copy link
Contributor Author

New failure:

make[3]: Entering directory '/d/bld/ipopt_1732388316225/work/build/src/Apps/AmplSolver'
../../../../src/Apps/AmplSolver/AmplTNLP.cpp:1775:25: warning: 'strdup' is deprecated: The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _strdup. See online help for details. [-Wdeprecated-declarations]
 1775 |       suftab_[i].name = strdup(suffix_ids_[i].c_str());
      |                         ^
>>> referenced by .libs/AmplTNLP.o:(public: void * __cdecl Ipopt::AmplOptionsList::Keywords(class Ipopt::SmartPtr<class Ipopt::OptionsList> const &, class Ipopt::SmartPtr<class Ipopt::Journalist const>, void **))

lld-link: error: undefined symbol: C_val_ASL
>>> referenced by .libs/AmplTNLP.o:(char * __cdecl Ipopt::get_str_opt(struct Option_Info *, struct keyword *, char *))
>>> referenced by .libs/AmplTNLP.o:(char * __cdecl Ipopt::get_haltonerror_opt(struct Option_Info *, struct keyword *, char *))

lld-link: error: undefined symbol: D_val_ASL
>>> referenced by .libs/AmplTNLP.o:(char * __cdecl Ipopt::get_num_opt(struct Option_Info *, struct keyword *, char *))

lld-link: error: undefined symbol: I_val_ASL
>>> referenced by .libs/AmplTNLP.o:(char * __cdecl Ipopt::get_int_opt(struct Option_Info *, struct keyword *, char *))
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

Probably there is something wrong on the ampl-asl Windows package.

@metab0t
Copy link

metab0t commented Nov 24, 2024

The asl.dll built on Windows does not export the symbols correctly.

@metab0t
Copy link

metab0t commented Nov 24, 2024

ASL symbols are now resolved, but strdup is still not found.

According to pybind/pybind11#3024, you can add three lines in src/Apps/AmplSolver/AmplTNLP.cpp as a patch.

#ifdef _WIN32
#define strdup _strdup
#endif

@traversaro
Copy link
Contributor Author

Thanks, I added a patch in the latest commit.

@traversaro
Copy link
Contributor Author

Linking now works, now the failure is:

  line>includedir=${prefix}/include/coin-or
 Variable declaration, 'includedir' has value 'D:/bld/ipopt_1732463061264/_test_env/Library/include/coin-or'
  line>
  line>Name: Ipopt
  line>Description: Interior Point Optimizer
  line>URL: https://github.com/coin-or/Ipopt
  line>Version: 3.14.16
  line>Cflags: -I${includedir}
  line>Libs: -L${libdir} -lipopt
  line>
  line>
Path position of 'ipopt' is 1
Adding 'ipopt' to list of known packages
find: 'Optimal Solution': No such file or directory
Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30157 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

cpp_example.cpp
MyNLP.cpp
Generating Code...
Microsoft (R) Incremental Linker Version 14.29.30157.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:cpp_example.exe 
ipopt-3.lib 
cpp_example.obj 
MyNLP.obj 
LINK : fatal error LNK1181: cannot open input file 'ipopt-3.lib'
WARNING: Tests failed for ipopt-3.14.16-hfa42872_10.conda - moving package to D:\bld\broken
Traceback (most recent call last):
  File "D:\Miniforge\Lib\site-packages\conda_build\build.py", line 3483, in test
    utils.check_call_env(
  File "D:\Miniforge\Lib\site-packages\conda_build\utils.py", line 404, in check_call_env
    return _func_defaulting_env_to_os_environ("call", *popenargs, **kwargs)

It seems that the import library are called ipopt.dll.lib or libipopt.la, while msvc/pkg-config expects ipopt-3.lib . I am not sure why this happens, I will try to replicate the build locally.

@traversaro
Copy link
Contributor Author

It seems that the import library are called ipopt.dll.lib or libipopt.la, while msvc/pkg-config expects ipopt-3.lib . I am not sure why this happens, I will try to replicate the build locally.

The renaming is probably happening in https://github.com/conda-forge/autotools_clang_conda-feedstock/blob/main/recipe/conda_build_wrapper.sh#L57, the problem is probably that the previous windows binary hardcoded the import library name to be libipopt-3.lib, but for coherence with official ipopt binaries and the installed pkg-config file it could make sense to just call it libipopt.lib.

@traversaro
Copy link
Contributor Author

Removing the hardcoded ipopt-3.lib from the tests, everything is working, that was less complicate then expected.

@traversaro traversaro changed the title [DRAFT] Switch to use autotools upstream build system also on Windows instead of vendored CMake [DRAFT] Switch to use autotools upstream build system also on Windows instead of vendored CMake and link ampl-asl also on Windows Nov 24, 2024
@traversaro
Copy link
Contributor Author

We also need to rename manually Library/lib/ipoptamplinterface.dll.lib and Library/lib/sipopt.dll.lib to Library/lib/ipoptamplinterface.lib and Library/lib/sipopt.lib, for compatibility with the installed .pc files.

@traversaro traversaro marked this pull request as draft November 24, 2024 20:49
@traversaro
Copy link
Contributor Author

This is ready to go, but to avoid any problem we can probably sync it with the future release of ipopt, see coin-or/Ipopt#806 .

@traversaro traversaro marked this pull request as ready for review December 14, 2024 15:43
@traversaro
Copy link
Contributor Author

This is ready to go, but to avoid any problem we can probably sync it with the future release of ipopt, see coin-or/Ipopt#806 .

The new version of ipopt was released, this PR is now superseded by #125 .

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

Successfully merging this pull request may close these issues.

There is no executable for IPOPT >= 3.13 on Windows
3 participants