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

Update package version to 3.14.17 and switch to upstream autools build system on Windows #125

Merged
merged 7 commits into from
Dec 16, 2024

Conversation

conda-forge-admin
Copy link
Contributor

@conda-forge-admin conda-forge-admin commented Dec 14, 2024

@traversaro comment:

This PR finally drops the vendored and custom mantained CMake build system used in this feedstock for Windows support, and switch to use the upstream build system. To avoid any possible ABI incompatibility, the change is done together with the bump to ipopt version 3.14.17.

As a byproduct, it is possible to know build the ipopt executable also on Windows, fixing #55 .

Important

The vendored Windows cmake build system used to name the ipopt import library as ipopt-3.lib, while the upstream is naming it ipopt.lib. If you use pkg-config to link to ipopt, you do not need to change anything, otherwise if you were hardcoding the name of the ipopt-3.lib import library, now you need to change it to ipopt.lib.
To provide a temporary backward compatibility, for ipopt 3.14.17 we continue to ship both ipopt.lib and ipopt-3.lib, but the ipopt-3.lib is considered deprecated (even if we cannot mark it as such in any way). The ipopt-3.lib installed import library will be removed in the next release of conda-forge package of ipopt, so please switch your downstream build scripts to either use pkg-config (that would work without modifications for both ipopt <= 3.14.16 and ipopt >= 3.14.17) or directly link ipopt.lib, thanks!

Original bot message:

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

I've started a version update as instructed in #124.

I'm currently searching for new versions and will update this PR shortly if I find one! Thank you for waiting!

Fixes #124

@conda-forge-admin
Copy link
Contributor Author

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.

…2, conda-smithy 3.45.0, and conda-forge-pinning 2024.12.14.08.05.48
@conda-forge-admin conda-forge-admin changed the title ENH: update package version ENH: update package version to 3.14.17 Dec 14, 2024
@conda-forge-admin conda-forge-admin marked this pull request as ready for review December 14, 2024 15:32
@traversaro traversaro changed the title ENH: update package version to 3.14.17 Update package version to 3.14.17 and switch to upstream autools build system on Windows Dec 14, 2024
@traversaro
Copy link
Contributor

@conda-forge-admin please rerender

@traversaro
Copy link
Contributor

traversaro commented Dec 14, 2024

@conda-forge/ipopt the PR is now ready for review, thanks!

@conda-forge-admin
Copy link
Contributor Author

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/12331121351. Examine the logs at this URL for more detail.

@traversaro
Copy link
Contributor

@conda-forge/ipopt the PR is now ready for review, thanks!

Actually not, now there is a Windows failure that was not there in #123, not sure if it is due to some change in ipopt :

configure: exit 1
sed: can't read libtool: No such file or directory
Traceback (most recent call last):

@traversaro
Copy link
Contributor

traversaro commented Dec 14, 2024

@conda-forge/ipopt the PR is now ready for review, thanks!

Actually not, now there is a Windows failure that was not there in #123, not sure if it is due to some change in ipopt :

configure: exit 1
sed: can't read libtool: No such file or directory
Traceback (most recent call last):

Related issue: conda-forge/autotools_clang_conda-feedstock#15 and conda-forge/autotools_clang_conda-feedstock#16 (comment) .

@traversaro
Copy link
Contributor

@traversaro
Copy link
Contributor

It seems that libtool is not found as the configure does not end successfully.

@traversaro
Copy link
Contributor

The step that fails is:

checking for clang++.exe option to enable C++11 features... none needed
checking dependency style of clang++.exe... none
checking whether clang++.exe understands -c and -o together... yes
checking for gfortran... no
checking for ifx... no
checking for ifort... no
checking for flang... flang
checking whether the compiler supports GNU Fortran 77... no
checking whether flang accepts -g... yes
checking how to get verbose linking output from flang... -v
checking for Fortran 77 libraries of flang...  -linker -libpath:D:/bld/ipopt_1734190797827/_h_env/Library/lib
configure: error: in '/d/bld/ipopt_1734190797827/work/build':
checking for dummy main to link with Fortran 77 libraries... unknown
configure: error: linking to Fortran libraries from C fails
See 'config.log' for more details
This file contains any messages produced by compilers while

The similar portion with 3.14.16 that was working is:

2024-11-25T08:20:32.0066991Z checking for gfortran... no
2024-11-25T08:20:32.0247930Z checking for ifort... no
2024-11-25T08:20:32.0434286Z checking for g95... no
2024-11-25T08:20:32.0625457Z checking for fort77... no
2024-11-25T08:20:32.0811726Z checking for f77... no
2024-11-25T08:20:32.0992767Z checking for f95... no
2024-11-25T08:20:32.1240417Z checking for f90... no
2024-11-25T08:20:32.1422756Z checking for g77... no
2024-11-25T08:20:32.1635680Z checking for pgf90... no
2024-11-25T08:20:32.1832578Z checking for pgf77... no
2024-11-25T08:20:32.2013594Z checking for ifc... no
2024-11-25T08:20:32.2193956Z checking for frt... no
2024-11-25T08:20:32.2373966Z checking for af77... no
2024-11-25T08:20:32.2552776Z checking for xlf_r... no
2024-11-25T08:20:32.2732359Z checking for fl32... no
2024-11-25T08:20:32.8837317Z checking whether the compiler supports GNU Fortran 77... no
2024-11-25T08:20:33.0881254Z checking whether  accepts -g... no
2024-11-25T08:20:33.0882856Z configure: No Fortran 77 compiler available.

So I guess the problem is coin-or/Ipopt@5bb5948 and coin-or/Ipopt@d63fe2c , that updated to a new autotools version (automake 1.7?) that probably added support for the new llvm flang, but it does not support the classic flang 5.0.0 old and patched version that we use in conda-forge, until the flang19 migration progress. As the previous ipopt build without fortran on Windows was passing fine and passing all the tests, I wonder if it just make sense to avoid installing a fortran compiler at all on Windows, at least for now.

@traversaro
Copy link
Contributor

I wonder if it just make sense to avoid installing a fortran compiler at all on Windows, at least for now.

To be honest, I am not even sure if fortran is doing anything on linux as well, as the runtime is not linked:

2024-12-14T15:43:41.6594853Z WARNING (ipopt): dso library package conda-forge/linux-64::libgfortran5==14.2.0=hd5240d6_1 in requirements/run but it is not used (i.e. it is overdepending or perhaps statically linked? If that is what you want then add it to `build/ignore_run_exports`)

@traversaro
Copy link
Contributor

I wonder if it just make sense to avoid installing a fortran compiler at all on Windows, at least for now.

To be honest, I am not even sure if fortran is doing anything on linux as well, as the runtime is not linked:

2024-12-14T15:43:41.6594853Z WARNING (ipopt): dso library package conda-forge/linux-64::libgfortran5==14.2.0=hd5240d6_1 in requirements/run but it is not used (i.e. it is overdepending or perhaps statically linked? If that is what you want then add it to `build/ignore_run_exports`)

Indeed, this seems the case according to ipopt docs (https://github.com/coin-or/Ipopt/blob/f7d4d416ae65b2a77cab2eb88ac2f45beb1df2f1/doc/main.dox#L200C1-L203C26):

In addition, the configuration script also searches for a Fortran
compiler. If all third party dependencies are available as self-contained
libraries and no Ipopt/Fortran interface needs to be build, a Fortran
compile is not necessary.

Indeed, it seems that fortran is only used to compile a few examples: https://github.com/coin-or/Ipopt/blob/f7d4d416ae65b2a77cab2eb88ac2f45beb1df2f1/configure.ac#L644-L660 .

@traversaro
Copy link
Contributor

I remove the dependency on a fortran compiler on Windows in 05d8c6a, I guess we can do the same on unix as well, but probably it may be worth to first test on Windows and if that works fine drop it also on Unix, I opened #126 to track this.

@traversaro
Copy link
Contributor

@conda-forge-admin please rerender

@traversaro
Copy link
Contributor

@conda-forge/ipopt the PR is now ready for review, thanks!

Actually not, now there is a Windows failure that was not there in #123, not sure if it is due to some change in ipopt :

configure: exit 1
sed: can't read libtool: No such file or directory
Traceback (most recent call last):

Problem solved, feedback on the PR is welcome!

@moorepants
Copy link
Contributor

Is it possible to dual support the ipopt-3.lib and ipopt.lib names to avoid breaking downstream builds?

@traversaro
Copy link
Contributor

Is it possible to dual support the ipopt-3.lib and ipopt.lib names to avoid breaking downstream builds?

Good point! I guess it should work to copy the import lib and everything should work fine, but I need to double check.

@traversaro
Copy link
Contributor

traversaro commented Dec 16, 2024

Ok, the modifications seems to work fine, the only doubt I have is that it seems that the import library for some reason is quite big (~1.6 MB):

2024-12-16T12:17:26.0568772Z 1.6M	/d/bld/ipopt_1734350876664/_h_env/Library/lib/ipopt.dll.lib

I would not be a big fan of forever shipping a duplicate import library, perhaps we can ship it in this release and announce that we will remove it in 3.14.18 ?

@moorepants
Copy link
Contributor

I would not be a big fan of forever shipping a duplicate import library, perhaps we can ship it in this release and announce that we will remove it in 3.14.18 ?

Thanks for doing that.

Ideally a downstream user would get a warning if they tried linking against ipopt-3.lib and then we remove it in the next release. But if there is no way to warn a person like that (other than making an issue here for people to find when linking breaks), then I guess it is no different than removing ipop-3.lib now. With that thought, I'm fine either way. Sorry for the churn.

@traversaro
Copy link
Contributor

I would not be a big fan of forever shipping a duplicate import library, perhaps we can ship it in this release and announce that we will remove it in 3.14.18 ?

Thanks for doing that.

Ideally a downstream user would get a warning if they tried linking against ipopt-3.lib and then we remove it in the next release. But if there is no way to warn a person like that (other than making an issue here for people to find when linking breaks), then I guess it is no different than removing ipop-3.lib now. With that thought, I'm fine either way. Sorry for the churn.

Actually I think that it make sense that we give a bit more of preparation time for downstream users, so for example in mechmotum/cyipopt#281 you have some time to address the problem while users can continue to build cyipopt with the new ipoipt. Clearly this is not useful unless a maintainer actually checks this repo for news, but even if it just simplify your (in the sense of cyipopt) life, I think it make sense to ship the backward compatible import library in 3.14.17 and then we remove it in 3.14.18 .

@traversaro
Copy link
Contributor

I modified the warning message in this PR. I also provided a pinned issue that contains the same message to maximize its visibility.

@traversaro
Copy link
Contributor

I modified the warning message in this PR. I also provided a pinned issue that contains the same message to maximize its visibility.

Done in #128 . I think it would be difficult to do more.

@moorepants
Copy link
Contributor

Thank you. All good from me.

@traversaro traversaro merged commit 4d78a5a into conda-forge:main Dec 16, 2024
9 checks passed
@traversaro
Copy link
Contributor

Thanks @moorepants !

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.

@conda-forge-admin please update version
3 participants