conda-forge / ctng-compilers-feedstock

A conda-smithy repository for ctng-compilers.
BSD 3-Clause "New" or "Revised" License
12 stars 25 forks source link

tzdb integration #142

Open h-vetinari opened 2 weeks ago

h-vetinari commented 2 weeks ago

Retry bits that were de-scoped from #133

conda-forge-webservices[bot] commented 2 weeks ago

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.

h-vetinari commented 2 weeks ago

OK, this now passes some basic tests, though I'm sure the implementation can be improved (e.g. less allocations as @mbargull pointed out in #133). It's still using the -ldl debug work-around (when we should really be adding libdl to the link interface of libstdc++.so (equivalent of CMake's target_link_libraries), but as I noted in #133:

I think it should be something like AC_LIB_LINKFLAGS, but the makefile(s) in GCC are just indecipherable for me (though I really tried to find where the shared lib gets defined...). Any ideas how/where to do this?

CC @conda-forge/ctng-compilers

h-vetinari commented 2 days ago

OK, so I made the variable static (also provides some caching if it has been loaded already), and managed to test this conclusively & successfully (see CI @ https://github.com/conda-forge/ctng-compilers-feedstock/pull/142/commits/108da784c0cfe886364b1e6306a75473ea9411dc)

I haven't managed to get rid of -ldl being for linking to libstdc++.so, I'm either going to need a tip there how to add this to the link interface through autotools, or alternatively, we could just add -ldl to the default C++ flags in the activation feedstock.

PTAL @isuruf @mbargull

h-vetinari commented 7 hours ago

I haven't managed to get rid of -ldl being for linking to libstdc++.so, I'm either going to need a tip there how to add this to the link interface through autotools, or alternatively, we could just add -ldl to the default C++ flags in the activation feedstock.

@isuruf, this still fails when we compile something that needs libstdc++, because the -ldl is not automatically added. I don't know how to "bake this in"; unless you have an idea how to achieve this, I propose to add -ldl to the activation feedstock. WDYT?

isuruf commented 7 hours ago

@isuruf, this still fails when we compile something that needs libstdc++, because the -ldl is not automatically added. I don't know how to "bake this in"; unless you have an idea how to achieve this, I propose to add -ldl to the activation feedstock. WDYT?

That's because libstdc++.so is not getting linked to libdl with my changes nor yours. Basically the patch you add doesn't work.

h-vetinari commented 7 hours ago

Basically the patch you add doesn't work.

That's not a big surprise to me; it was an exploratory change, because I saw -lm in there, and hoped that it would similarly translate. I stared at the makefiles for a while, but I cannot even make out where the .so is defined, nor do I know for sure that AC_LIB_LINKFLAGS would be the right approach if I did find it.

h-vetinari commented 7 hours ago

That's because libstdc++.so is not getting linked to libdl with my changes nor yours.

Though it is getting linked successfully while building with that change; before this patch, I needed to explicitly add -ldl in build.sh (see https://github.com/conda-forge/ctng-compilers-feedstock/pull/142/commits/c01b031eebc8f90c485c0c643ce0b2c432d983f4); after the patch that was not necessary anymore. However, that linkage information doesn't persist until after the build, when we're trying to link the test code with libstdc++

h-vetinari commented 7 hours ago

If I do patch the Makefiles again, is it important for you to use the -Bstatic -ldl -Bdynamic -Wl,--exclude-libs,libdl.a stuff, or was that just exploratory as well?

mbargull commented 6 hours ago

If I do patch the Makefiles again, is it important for you to use the -Bstatic -ldl -Bdynamic -Wl,--exclude-libs,libdl.a stuff, or was that just exploratory as well?

I haven't yet thought about the static linking case; I'll leave this one for @isuruf since he is more of an expert than me on it ;).