Skip to content

Conversation

seanyen
Copy link
Member

@seanyen seanyen commented Jun 13, 2019

This PR is to add a new export type Chocolatey for vcpkg.

With this PR, developers can do vcpkg export <packages> --x-chocolatey --x-maintainer=abc and it outputs Chocolatey packages in per-package manner. For example, if tinyexif is exported, then tinyexif.1.0.2.nupkg and tinyxml2.7.0.1.nupkg will be generated, because tinyexif depends on tinyxml2.

And on the import side, developers can copy over those packages and do choco install tinyexif -s <path of nupkg(s)>. In this example, tinyxml2 and tinyexif will be both installed to the current working directory, and the installed contents will be merged. Likewise, to remove packages, developers can do choco uninstall tinyexif tinyxml2 to remove both or do choco uninstall tinyexif to only remove tinyexif.

And why do I break a set of vcpkg installed snapshot into packages? I think that this modularization gives developers flexibility to pull down only the needed packages (and its dependencies) at the beginning, and they can pull down more as needed later.

@seanyen
Copy link
Member Author

seanyen commented Jun 13, 2019

@nuclearsandwich @ooeygui, any feedback is welcome!

@ooeygui
Copy link
Member

ooeygui commented Jun 13, 2019

👍

@ras0219-msft
Copy link
Collaborator

This is really cool!

I have a few questions/observations:

  1. How will this handle cases like updating boost from 1.65 to 1.66, then re-exporting cpprestsdk (note: cpprestsdk depends on boost)? If I understand correctly, this would generate a cpprestsdk package with identical version information and therefore conflict with the previous copy of that package.
    To fix this, I think you will need to add a mandatory prefix/suffix to either the package ID or the version string.
  2. How does this handle DLL deployment after? Inside vcpkg (and in the other monolithic exporters) we use our applocal.ps1 script to deploy DLLs to consuming projects. A requirement for that script to work is for all the vcpkg DLLs to live in the same folder. How do downstream consumers use the binaries produced by this exporter in their projects?

std::string maintainer = binary_paragraph.maintainer;
if (maintainer.empty())
{
maintainer = binary_paragraph.spec.name();
Copy link
Collaborator

@ras0219-msft ras0219-msft Jun 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to always set this maintainer field from a value specified by the exporter; some sort of vcpkg export --chocolatey --maintainer=abc.

This is because users who have chosen to be listed in the maintainer fields of the vcpkg ports probably don't want to be contacted if there are issues in a produced chocolatey package -- it should be the job of the chocolatey package producer to first determine whether the issue is an upstream problem and then forward to the listed maintainer.

Copy link
Member Author

@seanyen seanyen Jun 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I added it as CLI options in the latest iteration.

@ras0219-msft ras0219-msft added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Jun 14, 2019
@seanyen
Copy link
Member Author

seanyen commented Jun 18, 2019

  • How will this handle cases like updating boost from 1.65 to 1.66, then re-exporting cpprestsdk (note: cpprestsdk depends on boost)? If I understand correctly, this would generate a cpprestsdk package with identical version information and therefore conflict with the previous copy of that package.
    To fix this, I think you will need to add a mandatory prefix/suffix to either the package ID or the version string.

Good question! I added a option --version-suffix for users who decide to export the packages. I didn't restrict what format to use. It is up to the user who export the packages to decide what (for example, -20190618) to append to distinguish the rolling build version.

  • How does this handle DLL deployment after? Inside vcpkg (and in the other monolithic exporters) we use our applocal.ps1 script to deploy DLLs to consuming projects. A requirement for that script to work is for all the vcpkg DLLs to live in the same folder. How do downstream consumers use the binaries produced by this exporter in their projects?

Any pointers (docs) about the applocal.ps1 thing? Just want to make sure I understand what does it do. And for the usage, the primary motivation for this change is for ROS development, and the way I integrate vcpkg is to add vcpkg installed path to CMAKE_PREFIX_PATH for the ROS build tool catkin, so it can find the libraries & headers in a regular CMake way.

@seanyen
Copy link
Member Author

seanyen commented Jun 18, 2019

@lennonwoo @cottsay @nuclearsandwich JFYI.

@ras0219-msft
Copy link
Collaborator

Any pointers (docs) about the applocal.ps1 thing?

We don't have explicit docs other than the source code[2], which is a 100 line powershell script. Essentially, it recursively traverses the DLL dependency tree of an exe/dll and copies any DLLs needed out of vcpkg and into the build directory.

the way I integrate vcpkg is to add vcpkg installed path to CMAKE_PREFIX_PATH for the ROS build tool catkin, so it can find the libraries & headers in a regular CMake way.

Our documented and supported way to use packages is via our toolchain file[1], not via CMAKE_PREFIX_PATH. While CMAKE_PREFIX_PATH might work sometimes, it is not what we test and I can enumerate several examples where it simply will not work (and consuming projects will fail to build).

One way to fix this would be to export monolithic SDKs instead, that follow our existing --raw and --nuget schemes. In the context of ROS, I'd imagine this as a sub-sdk for each feature area and then one mega-sdk that has all the dependencies for the project.

Otherwise, given the above (that we don't have a supported use story for the packages after building), I think we can still include this as an "experimental+unsupported" feature. In this case, we need to add x- prefixes onto all relevant command lines.

[1] https://github.com/microsoft/vcpkg/blob/master/docs/users/integration.md#cmake-toolchain-file-recommended-for-open-source-cmake-projects
[2] https://github.com/microsoft/vcpkg/blob/master/scripts/buildsystems/msbuild/applocal.ps1

@seanyen seanyen changed the title [feature] add vcpkg export --chocolatey support [feature] add vcpkg export --x-chocolatey support Jun 19, 2019
@seanyen
Copy link
Member Author

seanyen commented Jun 19, 2019

Otherwise, given the above (that we don't have a supported use story for the packages after building), I think we can still include this as an "experimental+unsupported" feature. In this case, we need to add x- prefixes onto all relevant command lines.

I added x- to all the options (for example, --chocolatey becomes --x-chocolatey), and also added experimental feature to the help string. Hope that make itself clear enough for the experimental state of this feature. And it is good to hear Vcpkg can tolerate experimental feature and enable ROS community to iterate\explore more infrastructure work on Windows. 😄

nuspec_file_content =
Strings::replace_all(std::move(nuspec_file_content), "@PACKAGE_MAINTAINER@", chocolatey_options.maybe_maintainer.value_or(""));
nuspec_file_content =
Strings::replace_all(std::move(nuspec_file_content), "@PACKAGE_DESCRIPTION@", binary_paragraph.description);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the ports/boost-modular-build-helper/CONTROL file doesn't contain Description contents, and if we want export packages dependence on boost-modular-build-helper, the program will complain Description is required and exit, but not point out which package lacks the required information.

@ras0219-msft ras0219-msft merged commit 0791dbf into microsoft:master Nov 22, 2019
@ras0219-msft
Copy link
Collaborator

Thanks for making this PR @seanyen and sorry for the long delay!

If you would like to make changes to this in the future, please do feel free to open new PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants