[opennurbs] add new port

[Amin Ya (aminya)]

Describe the pull request

• What does your PR fix?

  Adds opennurbs

• Which triplets are supported/not supported? Have you updated the CI baseline?

all other than:
opennurbs:x64-uwp=fail
opennurbs:arm-uwp=fail

• Does your PR follow the maintainer guide?

  Yes

• If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

  Yes

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

No

[Amin Ya (aminya)]

I tested this on my Mac VM and it built successfully. I don't have access to the error logs that fail the CI. So, not sure how to debug this. Note that there is a uuid/uuid.h dependency that is handled inside the CMake file:
mcneel/opennurbs@bce8191

[Dale Fugier (dalefugier)]

Hey Billy O'Neal (@BillyONeal), I’ve created an issue, in our tracking system, so we can consider this.

https://mcneel.myjetbrains.com/youtrack/issue/RH-66553.

[Steve Baer (sbaer)]

Billy O'Neal (@BillyONeal) does making a project vcpkg compatible amount to getting it build with cmake or are there other requirements? We're getting hit a bit out of left field with a large pull request that I will not accept and want to better understand what is needed

[Robert Schumacher (ras0219-msft)]

It is not necessary to adopt CMake to build in vcpkg (we intend to support all C++ buildsystems), however there are other requirements:

1. No downloading during your build
2. No vendored dependencies -- third party libraries should be listed as dependencies in vcpkg and the library should build against the vcpkg versions. This doesn't mean you need to make your default "build from source" not use vendored dependencies, we just need an option to disable that behavior.

I'd like to suggest more, but I can't find a way to browse the OpenNURBS source online. What buildsystem do you currently use? We have existing examples for autotools, MSBuild, meson, bazel, qmake, as well as custom scripts.

(Also, thanks for reaching out! I'm really excited to have OpenNURBS available in vcpkg :))

[Dale Fugier (dalefugier)]

but I can't find a way to browse the OpenNURBS source online

Can you see this? - https://github.com/mcneel/opennurbs

[Steve Baer (sbaer)]

We have

• visual studio .sln/.vcxproj files for Windows compiles (see opennurbs_public.sln)
• xcode proj files for Mac
• makefile for everything else

[Robert Schumacher (ras0219-msft)]

Can you see...

I can! Sorry that I wasn't able to find it via search engine before :)

visual studio .sln/.vcxproj files for Windows compiles (see opennurbs_public.sln)
xcode proj files for Mac
makefile for everything else

Cool. We can handle .vcxproj via vcpkg_install_msbuild()1 and Makefile via vcpkg_build_make()2. I don't think we currently have any libraries that use XCode though; would it be possible to use the Makefiles for Mac?

We'll also need to patch out the checked-in copies of zlib/freetype in the build3, preferring to instead use the copies you get from vcpkg install zlib freetype. We can do this via a patch file, but it would be nice to instead have a simple make option that says "Hey, don't worry about the dependencies, we'll set up your CFLAGS/CCFLAGS/LDFLAGS such that they are available".

[Dale Fugier (dalefugier)]

Is patching zlib and freetype required? Currently, openNURBS does not use freetype . But it does use zlib. There are millions of opennurbs compressed meshes in 3dm data sets going back to Rhino 2.0 3dm files. This isn't a change we'd take lightly, needless to say.

[Robert Schumacher (ras0219-msft)]

Sorry, I think I wasn't clear enough.

We need to build OpenNURBS against a prebuilt copy of zlib instead of the copy checked in. That doesn't mean you need to delete it from your sources and it doesn't mean you need to make that even the default behavior of your buildsystem. We can easily make this change via a dozen or so lines of patching, which would simply remove the zlib objects from your lib and instead add -I/path/to/our/includes to your compile line and -L/path/to/our/libs -lz to your link line. You'll still have 100% of zlib available, so all your meshes and existing functionality will still work.

I see that there's a commented-out option to build with freetype -- is that deprecated functionality that just hasn't been fully removed yet?

[Steve Baer (sbaer)]

Our version of zlib is also very old. We do plan to update to a new version of zlib at some point, but we want to run through a large set of regression tests before even considering this. It is also something we don't have a high priority for doing at the moment since what we have works and we have many other projects that customers are more willing to pay us for.

[Robert Schumacher (ras0219-msft)]

That's fine -- customers using vcpkg accept and expect that they will be getting more recent versions of dependencies than what various projects were initially developed against. This is the same policy that essentially every Linux distribution follows; for example if Debian added OpenNURBS they would apply the same changes[1].

I want to also stress that not doing this is not an option -- if a user wants to use Boost and OpenNURBS at the same time, it is imperative that both Boost and OpenNURBS were built against the same copy of zlib. If each brought their own, that would violate the One Definition Rule, causing undefined behavior by the C/C++ standard.

If you are especially concerned, we can add a warning message during the build that explains to users about the situation and informs them that if they experience issues they should first reproduce them with the originally tested version of zlib before reporting upstream.

[1] https://www.debian.org/doc/debian-policy/ch-source.html#embedded-code-copies

[Amin Ya (aminya)]

I cherry-picked the CMake part of my pull request based on their request. Now, I am awaiting Robert McNeel & Associates (@mcneel) to review the smaller pull requests.
mcneel/opennurbs#29

[Steve Baer (sbaer)]

I'm pretty concerned about this rule and am not ready to just let anyone choose which version of zlib they want without us performing many tests. This IS our file format for Rhino and I am not willing to allow incorrect versions of our files to be created just so we can participate in vcpkg. It very well may not be an issue, but this requires thorough testing.

[Robert Schumacher (ras0219-msft)]

An alternative option that avoids ODR (but is more work and I wouldn't recommend) would be to give all the zlib symbols internal linkage. Since you compile as C++, this should actually be not terrible to do by creating a .cpp file with the contents:

namespace {
    #include "zlib.c"
    #include "zlib2.c"
    // the rest of the zlib implementation files
}

// declare your own functions with unique names (not the zlib ones!) that the rest of the application uses.

Then remove the individual zlib files from your buildsystem. This arrangement means that nobody outside your one cpp file will be able to see the zlib symbols, so it won't conflict with any other copies of zlib a user might link into their process. I notice that you already have a file opennurbs_zlib.cpp[1] which might be usable for this purpose.

This would meet our needs in vcpkg since our strict requirement is not violating ODR, though we'd still recommend adopting an external copy for better security, maintainability, and performance.

[1] https://github.com/mcneel/opennurbs/blob/7.x/opennurbs_zlib.cpp

[github-actions]

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

• ports/opennurbs/vcpkg.json

Valid values for the license field can be found in the documentation

[JonLiu1993]

Closing this PR since it seems that no progress is being made. Please reopen if work is still being done.

[Amin Ya (aminya)]

The upstream repo has been very slow to review the contributions.
mcneel/opennurbs#29

Should I add the CMake file to the port here?

[JonLiu1993]

The upstream repo has been very slow to review the contributions. mcneel/opennurbs#29

Should I add the CMake file to the port here?

Ok, after Upstream merges your dispatcher, we can delete Cmake in the process of updating the version later

[github-actions]

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

• ports/opennurbs/vcpkg.json

Valid values for the license field can be found in the documentation

[Zhao.MA (xarthurx)]

Is there any update on this PR?
It seems based on here: https://discourse.mcneel.com/t/opennurbs-pre-builds-for-github-action/146973/2
McNeel does not have a lot of interest in integrating opennurbs into vcpkg.

[Amin Ya (aminya)]

I'll add my changes (including the CMake support) as patch files in the following week.

[JonLiu1993]

Closing this PR since it seems that no progress is being made. Please ping us to reopen if work is still being done.

[Zhao.MA (xarthurx)]

Is there any updates on this topic?

[Steve Baer (sbaer)]

We have recently updated the zlib project that will ship with opennurbs in Rhino 9. This is the next step needed to allow for vcpkg support. We aren't planning on doing this for Rhino 8 version of opennurbs