-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[polyclipping] new port #6636
[polyclipping] new port #6636
Conversation
You probably want to patch the CMakeLists.txt of that port. It has something like: |
may I ask you why should I patch There are no config files produced by this build, it is common to have your own |
sry, I delete that sentence seconds after I posted it.
because it overwrites the build type or does it not? Argh CMakes (stupid) syntax rules. set(bla CACHE) only sets the value if it has not been supplied. Sry for that, was a bit too fast.
I find that a bit strange because it installs a package configuration file. Although it has not a lot of content. The port should probably be renamed to polyclipping because there are a lot of other packages called clipper and the first one on google with that name is actually not this port but this: https://github.com/ucbrise/clipper . Furthermore the project name in the CMakelists is polyclipping |
may I tell you one thing? You are usually too much fast at writing before documenting yourself. Also in the other PR about LibLZMA, which I am not linking because it's not a good practice to link PR that are not related (something that you do too much, also, spamming references without any reason), there are many broken statements by you, which I just avoided replying to reduce flame... please take your time and check things before spreading noise... otherwise again it's just more work for everybody, as I already told you. About config, there is a pkg-config file produced by CMake, not a native config, so usually it's not even considered during library consumption. Finally, about renaming, Clipper is how it is called and referenced. I'd not rename it if not requested by admins. In my google search page, which I am sure you know can be different from yours, this library is the first result, so that's not a valid metric. |
that is not 100% correct. There is no triplet supplied by vcpkg but users are free to change the linkage in the triplet or supply their own. We should also consider these users
there are already a few merged PRs by reverting that change. (A quick search through my port folders reveals 19 hits with only 1 removal of (CMAKE_)WINDOWS_EXPORT_ALL_SYMBOLS). I don't know if it is a good idea just an observation of mine. I probably would only force the static linkage on WIN32 and let somebody else figure out if WINDOWS_EXPORT_ALL_SYMBOLS is appropriate.
In hunter it is also referenced as polyclipping https://docs.hunter.sh/en/latest/packages/pkg/polyclipping.html?highlight=polyclipping
If I am wrong just correct me. I don't take it personally or as flame. Nobody knows everything and everybody can and will be wrong at some point. Correcting somebody is the faster experience because other might also wonder about that point and follow it. So please correct wrong arguments/assumptions. |
supported here means that there's a triplet supplied by the official master... About the remaining WINDOWS_EXPORT_ALL_SYMBOLS, I know them and together with @vicroms we decided to close that PR without touching them. It was already huge. Being the author of the two main commits in it I remember the anxiety I had, I can only imagine Victor's one. Fixing some small remaining one is a much lighter task and we left if for the expected subsequent PRs (only one in reality is requiring the re-addition of it, AFAIK, due to a big oversight I made) For the name, I'm open to discussion and change. But still, I know this library as clipper, I know I am not the only one so it would not be so strange for me if it was this one occupying this slot... |
Based on https://repology.org/project/clipper/versions and https://repology.org/project/polyclipping/versions, it does look like The pkgconfig/cmake-config files (if any) don't need to be renamed. |
ok I did it |
First part of a series, to resolve vxt regression related to openjpeg, which I cannot stand anymore 😆