-
Notifications
You must be signed in to change notification settings - Fork 18
Update BUILDING.md #54
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
Update BUILDING.md #54
Conversation
| 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`. |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
| ### 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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does
/std:c++latestand/or will/std:c++23cover these?
What are the "specific these" here?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| 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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Clarify build requirements and steps.