Skip to content
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

Windows installation via conda-forge #139

Merged
merged 16 commits into from
Jan 21, 2021
Merged

Conversation

mabelzhang
Copy link
Contributor

@mabelzhang mabelzhang commented Dec 23, 2020

Partially addresses gazebosim/docs#117

On Ubuntu, we have the distinction of using colcon for top-level compilation, and cmake and make manually for per-package compilation.
On Windows, those per-package steps up to cmake work,but make does not readily work. I think we'd need to call Visual Studio's compiler (cl?) directly. Instead of that, I currently put in the instructions for colcon for per-package as well, since we already know it works. If we figure out some standalone way later, we can change it.

Another reason for using colcon for per-package compilation on Windows is that we don't have binary builds for Windows. So users cannot optionally install, say, ign-* dependencies from binaries, and only compile the last ign- library from source. They would simply have to compile everything from source right now.

The plus side is that colcon will resolve all the dependencies automatically.

The downside is that the colcon workspace means that all the ign-* packages are coupled. So the READMEs for Windows won't look so clean like the Ubuntu sections. For Windows, we will have to refer to dependency ign-* packages and ask the user to install those first. At least that's what I believe; correct me if I'm wrong. Since ign-cmake is the lowest dependency, I've included all the setup for Conda and colcon workspace here, and will just refer to ign-cmake from other packages. That means they will not be very self-contained like Ubuntu ones.

Signed-off-by: Mabel Zhang <[email protected]>
@traversaro
Copy link
Contributor

On Windows, those per-package steps up to cmake work,but make does not readily work. I think we'd need to call Visual Studio's compiler (cl?)

CMake provides facilities to compile and install projects without caring for the specific build system used underneath, i.e. in place of make you can use:

cmake --build . --config Release 

to compile the project, and instead of make install you can use (available only since CMake 3.15, see https://stackoverflow.com/a/56457739):

cmake --install . --config Release

The --config Release is only useful when you use a Multiple-Config generator (such as Visual Studio or Xcode, see https://cgold.readthedocs.io/en/latest/glossary/multi-config.html) and it will be ignored when using single-config generators such as make or Ninja (see https://cgold.readthedocs.io/en/latest/glossary/single-config.html).

@@ -85,6 +95,45 @@ Build and install as follows:
make -j4
sudo make install

#### Windows

Open a Visual Studio Command Prompt (search for "x64 Native Tools Command Prompt for VS 2019" in the Windows search near the Start button).
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI PR conda-forge/vc-feedstock#19 added the package vs2019_win-64, that should source automatically the necessary variables of VS2019, but to be honest I tried it now and it is not working correctly for me, so probably sticking to this well known method is a good idea for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"feedstock" is a new word to me. Is the idea that with that package, Visual Studio can be installed via something like conda install vc vs2019_win-64, instead of from the Microsoft downloads? And then instead of using the VS Command Prompt, people can use the Anaconda prompt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the jargon. In conda, the "feedstock" is the repository that contains the metadata and the scripts to generate a conda package for a given library (see https://conda-forge.org/#about for more info).

The vs2019_win-64 conda package does not install Visual Studio 2019, but it just automatically invokes the script to set the environment variables set in the VS Command Prompt when a conda environment containg it is activated. However, as I was writing in the previous comment is not working correctly (at least for me). In any case if you use the Visual Studio 2019 16 Generator (as you are doing implicitly now in the docs), instead of the Ninja or NMake generator, it is not necessary to use it as CMake will handle all that configuration automatically.

Copy link
Contributor Author

@mabelzhang mabelzhang Dec 24, 2020

Choose a reason for hiding this comment

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

Thanks for the explanation! I understand half of that, but sounds good.

Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to ask for the point in which I was not clear. Re-reading it, I think it would be helpful to point out that the default CMake generator in Windows is the latest Visual Studio one if found, so following the instructions the build will default to use https://cmake.org/cmake/help/v3.19/generator/Visual%20Studio%2016%202019.html .

@traversaro
Copy link
Contributor

Another reason for using colcon for per-package compilation on Windows is that we don't have binary builds for Windows. So users cannot optionally install, say, ign-* dependencies from binaries, and only compile the last ign- library from source. They would simply have to compile everything from source right now.

Something that you could consider is to install also the ignition dependencies of a given ignition project from conda-forge itself. Some of them already have OR's maintainers, and I think existing maintainers (cc @wolfv @Tobias-Fischer) would be happy to add more mantainers from OR to those packages. Once a new release to a ignition package is done, a PR is automatically generated by the conda-forge infrastructure (see for example conda-forge/libignition-cmake0-feedstock#10), so releasing a new version in conda-forge in practice just means merging that PR, or not even doing that if you tell to the bot that it can auto-merge the release once the test are passing (see https://regro.github.io/cf-scripts/github_actions_infrastructure.html#automerging-prs).

@traversaro
Copy link
Contributor

By the way, thanks a lot @mabelzhang for doing this work in supporting Windows also in the docs, it is quite useful!

@chapulina chapulina added the Windows Windows support label Dec 23, 2020
Signed-off-by: Mabel Zhang <[email protected]>
@mabelzhang
Copy link
Contributor Author

mabelzhang commented Dec 23, 2020

Cool! Thank you @traversaro for the tips! I tried out the cmake commands and updated the tutorial accordingly.

I see the conda-forge/libignition-*-feedstock repos. I'll test out them out for subsequent packages (since ign-cmake doesn't have any dependencies).

Signed-off-by: Mabel Zhang <[email protected]>
@Tobias-Fischer
Copy link

Great to see this happening! As @traversaro mentioned I'm one of the maintainers of some ign packages on conda-forge. We'd be more than happy to add more maintainers and to merge any PRs that improve the feedstocks. Please open relevant PRs and we'll make sure to merge them timely 👍

@mabelzhang
Copy link
Contributor Author

Awesome thanks! conda-forge seems to make things amazingly easy.

Signed-off-by: Mabel Zhang <[email protected]>
Copy link

@JShep1 JShep1 left a comment

Choose a reason for hiding this comment

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

Looks good, just a few couple minor tweaks and it should be good.

tutorials/install.md Outdated Show resolved Hide resolved
tutorials/install.md Outdated Show resolved Hide resolved
tutorials/install.md Outdated Show resolved Hide resolved
tutorials/install.md Outdated Show resolved Hide resolved
tutorials/install.md Outdated Show resolved Hide resolved
tutorials/install.md Outdated Show resolved Hide resolved
tutorials/install.md Outdated Show resolved Hide resolved
tutorials/install.md Outdated Show resolved Hide resolved
@mabelzhang mabelzhang requested a review from JShep1 January 7, 2021 23:53
Copy link

@JShep1 JShep1 left a comment

Choose a reason for hiding this comment

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

LGTM!

tutorials/install.md Outdated Show resolved Hide resolved
@mabelzhang
Copy link
Contributor Author

mabelzhang commented Jan 14, 2021

I noticed that the tutorial has a Documentation and a Testing section... which in other libraries have been living in README instead of the tutorial. Currently, those sections are also present in the README, but are 7 months old. So I think these ones are more recent.

Do we want to move these back to the README, or delete them in README and move Documentation and Testing in other Ignition libraries into their installation tutorials for consistency?

@JShep1
Copy link

JShep1 commented Jan 20, 2021

Do we want to move these back to the README, or delete them in README and move Documentation and Testing in other Ignition libraries into their installation tutorials for consistency?

I moved the documentation and testing sections to the README d88bce5

Signed-off-by: John Shepherd <[email protected]>
@JShep1
Copy link

JShep1 commented Jan 20, 2021

I moved the documentation and testing sections to the README d88bce5

Reverted in 6da496c as we're now moving the documentation and testing sections to the install file as it has more context for the new user.

John Shepherd added 2 commits January 20, 2021 12:55
nit
Signed-off-by: John Shepherd <[email protected]>
Signed-off-by: John Shepherd <[email protected]>
@JShep1 JShep1 merged commit f78d69e into ign-cmake2 Jan 21, 2021
@JShep1 JShep1 deleted the mabelzhang/windows_install branch January 21, 2021 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice Windows Windows support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants