Skip to content

Conversation

@GabrielDosReis
Copy link
Collaborator

Clarify build requirements and steps.

@GabrielDosReis GabrielDosReis merged commit 294a10a into microsoft:main Oct 30, 2023
@GabrielDosReis GabrielDosReis deleted the gdr/update-BUILDING branch October 30, 2023 22:39
Comment on lines +16 to +20
On Windows platforms, we recommend using [vckpg](https://vcpkg.io/en/getting-started.html) as the C++ package manager. Issue
```sh
$ ./vckpg install --triplet x64-windows ms-gsl
```
in the VCPKG directory, to install the GSL. The above command assumes you are building for the `x64` target. Replace that with the appropriate triplet if you are taregting a different platform like `arm64`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This suggests that vcpkg is somehow not cross-platform, which is very far from the truth. Manifest mode is also recommended over classic mode: https://learn.microsoft.com/en-us/vcpkg/users/classic-mode

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This suggests that vcpkg is somehow not cross-platform,

Hmm. Walk me slowly through the logical steps so I can arrive at the same conclusion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nowadays it's fortunately rare, but you can still occasionally find people who think that think vcpkg is a Windows only product merely by coming from Microsoft and I don't think the wording here helps that. I have personally never used classic mode, so I don't know the quality of user experience of that on *nix systems. Manifest mode fortunately makes it very clear that it's for cross-platform use by the way it integrates with CMake.

I also think it's there's too much info about vcpkg here. Manifest mode is very simple and it's clearly documented with the CI workflows serving as an example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nowadays it's fortunately rare, but you can still occasionally find people who think that think vcpkg is a Windows only product merely by coming from Microsoft and I don't think the wording here helps that.

How would you have phrased it? Keep in mind that this project does not require VCPKG and it is not a goal. Nor it is its mission to do VCPKG advocacy, or make it vcpkg-centric. It will take advantage of it where available.

I think using classic mode is fine, even if someone of us don't personally use them.

Manifest mode is very simple and it's clearly documented with the CI workflows serving as an example.

That might be true but the goal here isn't to be as minimal as possible, but to get people quickly started. Repetition in that context isn't necessarily bad. The experts will ignore those, and that is OK.

Copy link
Contributor

Choose a reason for hiding this comment

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

If minimalism is preferable, I'd only document manifest mode, since that only requires passing one (two if you count target triplet) additional flag to CMake.

I'd also not mention vcpkg in an OS specific context. The users will have to figure out how to fit things into their system either way or they can rely on the software distribution on their Linux distro of choice once 0.43.1 is tagged. The prior is highly context sensitive, so not worth documenting here at all.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If minimalism is preferable, I'd only document manifest mode, since that only requires passing one (two if you count target triplet) additional flag to CMake.

Like I said, the goal of this document isn't to be as minimal as possible - but rather how to get people started as quickly as possible. While we can require them to sift through several links or documentations, we might as well make it easier for them to get started with this project. I updated the doc after I got more requests for help to get started (and we have a public record of some newcomers getting confused with terse instructions).

The users will have to figure out how to fit things into their system either way

Yes, they will have. There is value in facilitating that.

Is the update to this doc causing any actual confusion?

Comment on lines -25 to -29
### Building with MSVC

If you are building this project with MSVC, you need to pass some flags (like `/permissive-` and other `/Zc` switches) to requires ISO C++ conformant behavior. See the `flags-windows` preset in the
[CMakePresets.json](CMakePresets.json) file for the flags and with what
variable to provide them to CMake during configuration.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does /std:c++latest and/or will /std:c++23 cover these?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does /std:c++latest and/or will /std:c++23 cover these?

What are the "specific these" here?

Copy link
Contributor

Choose a reason for hiding this comment

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

The flags that this paragraph calls the users' attention to. MSVC by default is not ISO C++ conforming and I believe this is an important detail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The flags that this paragraph calls the users' attention to. MSVC by default is not ISO C++ conforming and I believe this is an important detail.

The instructions here are for building the project. That paragraph does not add anything because the actual flags are in the CMakePresets.json.

Comment on lines +22 to +30
On linux platforms, there is no commonly agreed-upon name for that package. Here are a few common name for the Microsoft implementation of the GSL on some linux distributions if you're using the system package manager:

- arch: `microsoft-gsl`
- debian: `ms-gsl`
- fedora: `guidelines-support-library`
- gentoo: `dev-cpp/ms-gsl`
- ubuntu: `ms-gsl`

Please, consult your linux distribution manual to find the appropriate package name.
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be better to upstream to the GSL readme and just refer to that instead.

Copy link
Collaborator Author

@GabrielDosReis GabrielDosReis Nov 3, 2023

Choose a reason for hiding this comment

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

This might be better to upstream to the GSL readme and just refer to that instead.

This project has a dependency on a component and it is providing information in how to satisfy that dependency. I would like to have the instructions for building this project to be approachable and relatively self contained to get started. If some of the information here could be also useful somewhere else, that is great, but that shouldn't prevent us from making it easy to quickly get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants