conda-forge / libtiff-feedstock

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

Shared library filename changed from `libtiff.dll` to `tiff.dll` #35

Closed djhoese closed 5 years ago

djhoese commented 5 years ago

I've been debugging some issues with gdal coming from conda-forge as of about two days ago. I tracked it down to a libtiff change. See https://github.com/conda-forge/gdal-feedstock/issues/241 for more debugging details on this. Bottom line is that the _gdal*.pyd file is trying to link against libtiff.dll, but for the last couple builds of libtiff this file is called tiff.dll. If I copy tiff.dll to libtiff.dll on both my Windows 7 VM and on my appveyor environment I am able to import gdal successfully by doing:

python -c "from osgeo import gdal, osr"

I downloaded the last couple windows tarballs from anaconda.org and get:

$ ls -1 Library-4.0.9-*/bin/*tiff.dll
Library-4.0.9-h6ce49d1_0/bin/libtiff.dll
Library-4.0.9-he490001_1002/bin/tiff.dll
Library-4.0.9-he490001_2/bin/tiff.dll
Library-4.0.9-vc9_0/bin/libtiff.dll

My guess is that the vc9/vc14 builds of gdal have been depending on the vc9/vc14 builds of libtiff for a long time (libtiff.dll), but with recent rebuilds of gdal they are now using the newest compilers and build environments and are now using the newer builds of libtiff (tiff.dll) which is why no one has noticed this until now. Again this is just a guess and is coming from someone who doesn't build things on Windows usually.

djhoese commented 5 years ago

@ocefpaf Looks like ever since PR #30 the .dll is named tiff.dll instead of the expected libtiff.dll.

Edit: Relevant? https://github.com/conda-forge/libtiff-feedstock/pull/30/files#diff-d04c86b6bb20341f5f7c53165501a393L20

ocefpaf commented 5 years ago

@ocefpaf Looks like ever since PR #30 the .dll is named tiff.dll instead of the expected libtiff.dll.

Edit: Relevant? conda-forge/libtiff-feedstock/pull/30/files#diff-d04c86b6bb20341f5f7c53165501a393L20

Awesome detective work! Thanks!! I'll try to fix those tomorrow if no one beats me to it. I guess that we can either add that workaround back or point gdal to the "right" place.

ocefpaf commented 5 years ago

https://github.com/conda-forge/gdal-feedstock/pull/243/commits/4edce94be199530bb440e0ca0883d8112e51e645 and https://github.com/conda-forge/geotiff-feedstock/pull/10/commits/0a2d4d2aa91fd11f4d9b6bd3d23c108fbae5d9a2 should fix this. Closing it b/c the fix is not really in this feedstock.

djhoese commented 5 years ago

@ocefpaf I'm confused by these fixes. Doesn't this mean that every package that depends on libtiff has to be updated? Why is tiff.dll the proper name for this when it seems every other platform uses libtiff.so (or is that a Windows thing)? Isn't the magic libtiff name change the issue? Or is this something that has been wrong in conda/conda-forge's recipes since libtiff was first created?

ocefpaf commented 5 years ago

@ocefpaf I'm confused by these fixes. Doesn't this mean that every package that depends on libtiff has to be updated?

Unfortunately, yes.

Why is tiff.dll the proper name for this when it seems every other platform uses libtiff.so (or is that a Windows thing)?

Proper or no that is the the designed name, we removed a "workaround" when we removed the renaming.

Isn't the magic libtiff name change the issue? Or is this something that has been wrong in conda/conda-forge's recipes since libtiff was first created?

Probably. I don't recall why we copied tiff.dll to libtiff.dll. Either way, it is better to be consistent with other channels to avoid more incompatibilities down the road. (Even though that means some rebuilds and broken packages for a while.)

djhoese commented 5 years ago

Sounds good. Thanks for the info.

jakirkham commented 5 years ago

Usually on Windows one does not prefix library names with lib*. It's likely downstream libraries like gdal were looking for this name, which caused us to name it with the lib* prefix here. This may have been convenient for whatever reason at the time (e.g. less work when people were swamped). Though ultimately agree moving to the designated name and alignment with defaults is the right move.

Raised issue ( https://github.com/regro/cf-scripts/issues/416 ) to see if we can get some help with the needed rebuild.

hobu commented 5 years ago

This change in the name of consistency that breaks everyone is a bit unfortunate.

Usually on Windows one does not prefix library names with lib*.

Yes, this is true.

It's likely downstream libraries like gdal were looking for this name, which caused us to name it with the lib* prefix here.

Yes, this is also true. I believe the history of GDAL and friends using libtiff.dll for windows instead of tiff.dll for linking is Adobe and other softwares had a tiff.dll that often name clashed.

ocefpaf commented 5 years ago

Yes, this is also true. I believe the history of GDAL and friends using libtiff.dll for windows instead of tiff.dll for linking is Adobe and other softwares had a tiff.dll that often name clashed.

Interesting, still. by doing that we are going against the original name of the library. I'm curious to know what @mingwandroid thinks about this b/c the original change that removed the libtiff name is from the AR recipe.

mingwandroid commented 5 years ago

We don't generally worry about name clashes with Adobe or anything else for that matter unless the DLL is getting installed in C:\Windows\System32 (and then we're sort of forced to do something about it or we have to recommend users to delete those DLLs).

I think our gdal recipes have diverged quite a bit now, but I'm not in any position to help out at present and won't be for a few weeks at least.

Sorry all!

mingwandroid commented 5 years ago

lib prefixes often appear on Windows, but I have little recollection of this change. I'm too groggy and not using computers for a while!

I don't see in the linked PR where the name is changed or the lib prefix gets set. How about we just have both as a hack? Copy libtiff.dll to tiff.dll?

ocefpaf commented 5 years ago

I think our gdal recipes have diverged quite a bit now

Actually that is not true. The issue is b/c we merged the recipes :-)

but I'm not in any position to help out at present and won't be for a few weeks at least.

Don't worry. Knowing that that DLL class name is not an issue we will keep this change and start rebuild stuff. Thanks!

How about we just have both as a hack? Copy libtiff.dll to tiff.dll?

That is what we had before.

hobu commented 5 years ago

The lib* naming convention on Windows notwithstanding, it is generally known that "libtiff.dll" is the libtiff.org library and not any possible permutation of TIFF-reading software that might otherwise be stored in a "tiff.dll" (including the Adobe-built? one that sometimes used to cause clashes). I believe this convention has been around since some of the first builds of libtiff on windows and their use by GDAL and any downstream consumers (since the late 90s).

I don't understand the value a change like this brings other than to ripple breakage up and down the dependency chain while we await things to regenerate and leave a latent breakage for when someone's alternative tiff.dll actually does clash. The default DLLNAME in nmake.opt of the libtiff source tree is libtiff.dll instead of tiff.dll and has been for nearly two decades. Sometimes things are named the way they are for a reason, even if they break a convention.

As a software project (PDAL) who has been investing Conda packaging, I find these kinds of changes extremely discouraging. Our package installs have been broken for days (for doing the simple conda install -c conda-forge pdal install without a manual pin of the libtiff).

ocefpaf commented 5 years ago

The lib* naming convention on Windows notwithstanding, it is generally known that "libtiff.dll" is the libtiff.org library and not any possible permutation of TIFF-reading software that might otherwise be stored in a "tiff.dll" (including the Adobe-built? one that sometimes used to cause clashes). I believe this convention has been around since some of the first builds of libtiff on windows and their use by GDAL and any downstream consumers (since the late 90s).

This is our build script on Windows,

https://github.com/conda-forge/libtiff-feedstock/blob/master/recipe/bld.bat

as you can see it is very simple and whatever is configured in the cmake files will be used to create libraries with the intended from upstream. My guess is that this convention you are referring to probably never made upstream then. Also, as @mingwandroid mentioned above, there won't be a clash with Adobe in conda.

I don't understand the value a change like this brings other than to ripple breakage up and down the dependency chain while we await things to regenerate and leave a latent breakage for when someone's alternative tiff.dll actually does clash. The default DLLNAME in nmake.opt of the libtiff source tree is libtiff.dll instead of tiff.dll and has been for nearly two decades. Sometimes things are named the way they are for a reason, even if they break a convention.

The value is to be consistent with defaults, who triggered the change in

https://github.com/AnacondaRecipes/libtiff-feedstock/commit/1e2c29894b78f279781a7192dcf21966cfb59c9b#diff-d04c86b6bb20341f5f7c53165501a393

I'm pinging @msarahan and @mingwandroid who managed that recipe there and probably can answer your question from the packaging side, from conda-forge side we only ant to avoid headaches and incompatibilities between thw two channels.

BTW, can you send any references for the convention you are mentioning? Are there any references of it upstream in libtiff? Or a PR to libtiff cmake config files?

As a software project (PDAL) who has been investing Conda packaging, I find these kinds of changes extremely discouraging. Our package installs have been broken for days (for doing the simple conda install -c conda-forge pdal install without a manual pin of the libtiff).

I understand your frustration but note that ee are all volunteers in conda-forge and I was away this past week, so it took me a while to remove the old builds that had libtiff names on them and caused you grief. I just did that and, from now on, all builds should use the default name that comes from the standard cmake build.

Note that I'm not against reverting this to the old name style but I won't do that unless the default channel is OK with it too b/c I don't want to cause more incompatibilities between the to channels.

mingwandroid commented 5 years ago

We need to fix it back to tiff.dll, this was unintentional as @ocefpaf points out.

But to ensure we don't break things we need to copy it to libtiff.dll too.

We definitely need a process to check for such breakages in future though. Debian uses libabigail for this I think and we need something similar.

I had to stop using the prebaked makefiles because they didn't work correctly. I cannot remember the exact details of that though.

mingwandroid commented 5 years ago

I didn't mention Adobe here. Do they put DLLs in C:\Windows\System32? I hope not, but if so then we should append something to the DLL name that distinguishes that this was built by AD/conda-forge then we need to rebuild all downstreams again.

But the most critical thing here is to get a check in place somewhere that checks for ABI breakage against the previous version.

hobu commented 5 years ago

The value is to be consistent with defaults, who triggered the change in AnacondaRecipes@1e2c298#diff-d04c86b6bb20341f5f7c53165501a393

Ok, my complaint is with that, not conda-forge trying to align with it.

I didn't mention Adobe here. Do they put DLLs in C:\Windows\System32?

I vaguely remember a name clash thing. I don't know if it was particularly Adobe's build or someone else's that was called tiff.dll, but at one time there was a common naming clash. The convention that libtiff.dll was always the stock libtiff.org dll had stuck after a while and no one had ever really modified it unless they were doing something special for their own application. A long time ago, badly behaving applications could do things like stick DLLs in system32 to ensure they were in the path, but Windows prevents that nowadays.

Many of the other open source geospatial projects do not follow the standard windows DLL naming convention in their default configurations (is this a hard-written rule anywhere, btw?), and if there are to be any more harmonization, this kind of breakage is very likely to happen for a number of projects coming out of that realm.

mingwandroid commented 5 years ago

As I said, the change wasn't deliberate. You see no mention whatsoever of lib in that commit, so the bug is an upstream one and should be reported and fixed there.

and if there are to be any more harmonization, this kind of breakage is very likely to happen for a number of projects coming out of that realm.

Please stop making inflammatory comments. I also said we need technical measures in place to prevent such things. PRs welcome for that, for that's how Open Source works best.

A long time ago, badly behaving applications could do things like stick DLLs in system32 to ensure they were in the path, but Windows prevents that nowadays.

Unfortunately it does not. Any installer run with administrative rights can put DLLs in this directory. See Amplitube as a constant thorn in my side wrt MKL DLLs. Also they do not end up on PATH, just in the DLL search path as determined by the Windows loader.

is this a hard-written rule anywhere, btw?

No such thing exists.

hobu commented 5 years ago

Please stop making inflammatory comments.

It was not clear to me throughout this discussion that the change upstream to rename to tiff.dll was not a deliberate change. Is it also not generally a Conda policy to rename project DLLs even though they don't follow the unwritten library naming convention on windows? PDAL's certainly don't, for example.

Any installer run with administrative rights can put DLLs in this directory.

I was confusing with OSX, which has the System Integrity Protection stuff.

jakirkham commented 5 years ago

We need to fix it back to tiff.dll...

But to ensure we don't break things we need to copy it to libtiff.dll too.

Did I understand this correctly, @mingwandroid? My understanding is you are proposing we include both tiff.dll and libtiff.dll? Is that right? If so, that sounds like the fastest path to resolving the breakage.

mingwandroid commented 5 years ago

@hobu conda-forge and Anaconda Distribution are deeply symbiotic endeavours. Both are composed of many software packaging experts. Combining our efforts is an explicit goal. We are working very hard to achieve compatibility on many fronts so your suggestion that we stop this is unhelpful. I explained already that it wasn't a deliberate change here: https://github.com/conda-forge/libtiff-feedstock/issues/35#issuecomment-445245564

@jakirkham We should ask upstream what they want the DLL to be named. If we don't provide both then Anaconda Distribution will need to rebuild all things that link to libtiff.dll.

Apologies for this mistake!

hobu commented 5 years ago

so your suggestion that we stop this is unhelpful.

The 'harmonization' I was referring to was trying to bring all DLL names into harmony with the unwritten windows library naming rules despite what upstream projects might have named them. That's not what you're trying to do. I was not trying to push against conda-forge <-> Anaconda harmony.

mingwandroid commented 5 years ago

Ok. I guess you did miss where I said it wasn't deliberate then. No problem.

mingwandroid commented 5 years ago

We should try to pass -DCMAKE_LIB_PREFIX= here on Windows to fix it, that will ensure the .lib doesn't contain libtiff.dll either.

jakirkham commented 5 years ago

@mingwandroid, unfortunately raising with upstream might be problematic. The homepage listed in this feedstock is parked and a mirror is being used for the source tarballs. Not really sure where else we should go instead.

hobu commented 5 years ago

@jakirkham by upstream, do you mean upstream of libtiff itself? ~If you file bugs in the GDAL repository, we can get things upstreamed to the actual libtiff distribution.~ Traffic at https://gitlab.com/libtiff/libtiff will make it into releases. Releases are roughly annually, and there just was one, so any patches won't show up for a very long time.

jakirkham commented 5 years ago

Meaning libtiff. So maybe the information in this feedstock is out-of-date. Where is libtiff currently being maintained? Any ideas what the homepage should be referencing?

hobu commented 5 years ago

libtiff is managed at https://gitlab.com/libtiff/libtiff