[freetds] Update to v1.2.5 and update source to github. #14120
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes failures in CI due to freetds.org being gone.
BillyONeal
added
category:port-bug
JackBoosY
approved these changes
Oct 20, 2020
BillyONeal
changed the title
|
@freddy77 Do you have any comments on this? |
BillyONeal
force-pushed
the
update_freetds
branch
from
da1efb2 to
17ea1ee
Compare
|
It seems to me the final result is worst than before. They seem just some quick hacks to make it compile. Nothing wrong as temporary fix but I would look for a better final solution |
Can you be more specific on how it is worse?
Yes, that's what our patches are in the business of doing :)
Right, that's why I tagged you here since you seem to be the most recent contributor to the upstream repo and it would be nice to distil this down into something you might actually want to merge there for these 2 changes (adding an option to skip building the unit tests, and making the cmake build system actually correct with respect to encoding.h) |
Yes,
What's wrong with |
That's how CMake does this. If you don't make it a target then there is no dependency ordering so nothing forces that
That is intentional; libraries' users don't need or care about them: https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/maintainer-guide.md#do-not-build-testsdocsexamples-by-default
Pool part?
Yes, if you have enough cores, cmake tries to build things that need encoding.h before encoding.h is created in the upstream build system. That doesn't happen with the tar.gz because there you have minted encoding.h in advance. |
Understood. Trying something more complete, see FreeTDS/freetds@f835b99
Mumble... with Autoconf a simple
|
Ah, I see, you have EXCLUDE_FROM_ALL'd them, so that patch might indeed be unnecessary.
Ah, I did that because I think we are just deleting the output of those targets, both before and after this change; for example vcpkg/ports/freetds/portfile.cmake Line 44 in 180cae5
That seems to have made the problem worse, since now iconv_charsets.h isn't being treated as a dependency of the tds port after that change since it isn't generated under that current directory. On my 64 thread system with vcpkg targeted at that commit SHA and all patches removed now I get: [,..] |
Not that would kill compiling a couple of more files.
Fixed, my mistake, see FreeTDS/freetds@d336746 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes failures in CI due to freetds.org being gone.