Skip to content

build2 integration doc update #4587

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

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from
Draft

Conversation

Klaim
Copy link
Contributor

@Klaim Klaim commented Jan 7, 2025

This PR adds a more complete description of how to use the build2's package of nlhomann-json to consume the library in build2 projects.

Note that build2 is not only a package-manager but also a build-system and both tools are designed to work together, this impacts the verbosity of this documentation. I might have been too detailed or beginner-friendly in some parts, feel free to point if I should shortcut some of the explanations. In doubt, for the case of exising build2 projects, I opted to use the "detail/summary" markdown/html tags so that people who already know about these steps (experimented build2 users) dont have to read the whole paragraph. The details are for less experimented build2 users.

The provided source files do not constitute as provided a valide build2 project, that involves a specific organisation in particular for projects that use the package-manager. So instead of adding a whole project, I put there the files that will be modified from a bdep new-created project (that command creates a ready-to-work project). The example "from scratch" uses that approach.

Because build2 projects with executables usually use testscript , the executable I/O testing tooling provided by build2, I also provided a testscript for testing the output from the classic output of the example source code.


Pull request checklist

Read the Contribution Guidelines for detailed information.

  • Changes are described in the pull request, or an existing issue is referenced.
  • The test suite compiles and runs without error.
  • Code coverage is 100%. Test cases can be added by editing the test suite.
  • The source code is amalgamated; that is, after making changes to the sources in the include/nlohmann directory, run make amalgamate to create the single-header files single_include/nlohmann/json.hpp and single_include/nlohmann/json_fwd.hpp. The whole process is described here.

Please don't

  • The C++11 support varies between different compilers and versions. Please note the list of supported compilers. Some compilers like GCC 4.7 (and earlier), Clang 3.3 (and earlier), or Microsoft Visual Studio 13.0 and earlier are known not to work due to missing or incomplete C++11 support. Please refrain from proposing changes that work around these compiler's limitations with #ifdefs or other means.
  • Specifically, I am aware of compilation problems with Microsoft Visual Studio (there even is an issue label for this kind of bug). I understand that even in 2016, complete C++11 support isn't there yet. But please also understand that I do not want to drop features or uglify the code just to make Microsoft's sub-standard compiler happy. The past has shown that there are ways to express the functionality such that the code compiles with the most recent MSVC - unfortunately, this is not the main objective of the project.
  • Please refrain from proposing changes that would break JSON conformance. If you propose a conformant extension of JSON to be supported by the library, please motivate this extension.
  • Please do not open pull requests that address multiple issues.

@Klaim
Copy link
Contributor Author

Klaim commented Jan 7, 2025

@nlohmann Hi! I'm not done at all with this but I would need to see the result somewhere before continuing. Does the ci makes a documentation build? I didnt find it so far.

@nlohmann
Copy link
Owner

nlohmann commented Jan 7, 2025

@nlohmann Hi! I'm not done at all with this but I would need to see the result somewhere before continuing. Does the ci makes a documentation build? I didnt find it so far.

You can execute

make install_venv serve -C docs/mkdocs

and then open http://127.0.0.1:8000/

@coveralls
Copy link

coveralls commented Jan 7, 2025

Coverage Status

coverage: 99.186%. remained the same
when pulling 71a98d9 on Klaim:build2-usage
into 93e9573 on nlohmann:develop.

Copy link

This pull request has been marked as stale because it has had no activity for 30 days. While we won’t close it automatically, we encourage you to update or comment if it is still relevant. Keeping pull requests active and up-to-date helps us review and merge changes more efficiently. Thank you for your contributions!

@github-actions github-actions bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Feb 14, 2025
@Klaim
Copy link
Contributor Author

Klaim commented Feb 14, 2025

Still relevant, just no time to focus on this yet. T_T I'll try this weekend but can't make any promises.

@nlohmann
Copy link
Owner

Please resolve conflicts and sign off your commits (see https://github.com/nlohmann/json/pull/4587/checks?check_run_id=35268369393).

@nlohmann nlohmann added the please rebase Please rebase your branch to origin/develop label Apr 12, 2025
@Klaim Klaim force-pushed the build2-usage branch 3 times, most recently from b3f711e to 7b12a7b Compare April 12, 2025 23:48
@Klaim
Copy link
Contributor Author

Klaim commented Apr 12, 2025

I took some time to update this but I'm having some issues:

  • I can't seem to satisfy DCO, could you point me to my mistake? I did add the signature to the commits.
  • The static code analysis are applied on example code, is this normal? Should I ignore it?
  • My code editor removed a bunch of trailing whitespaces automatically, we can see them in the changes. Should I revert that or is this ok?

I'll try to complete this soon, I didnt try to generate the doc yet by lack of time (and no make installed on this machine) but plan to do so in the coming days.

@@ -363,7 +363,7 @@ and follow the then displayed descriptions. Please see the vcpkg project for any
```cmake title="CMakeLists.txt"
--8<-- "integration/vcpkg/CMakeLists.txt"
```

Copy link
Owner

Choose a reason for hiding this comment

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

Please revert the whitespace changes in this file.

Nevertheless, after finding the package you want, click on “Install” button and accept confirmation dialogs.
After the package is successfully added to the projects, you should be able to build and execute the project

Nevertheless, after finding the package you want, just click on “Install” button and accept confirmation dialogs.
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove "just" - it's never "just".

After the package is successfully added to the projects, you should be able to build and execute the project

Nevertheless, after finding the package you want, just click on “Install” button and accept confirmation dialogs.
After the package is successfully added to the projects, you should be able to just build and execute the project
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove "just".

depends: * bpkg >= 0.16.0
#depends: libhello ^1.0.0

depends: nlohmann-json ^3.0.0
Copy link
Owner

Choose a reason for hiding this comment

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

I think depending from the latest version makes more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As long as the last version is 3.x this will acquire the last published version (^ means any version starting with the major version specified). If we really want to specify any last version then I can remove the version constraint, but it's usually not a good idea.

Copy link
Owner

Choose a reason for hiding this comment

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

Given that we do not release frequently, and releases usually fix a lot of bugs, using the latest version is best practice.

I can bump the version number with a new release.

@@ -0,0 +1,2 @@
The files in this directory are for documentation only. In the current file/directory configuration they cannot be used as a `build2` project. Use `bdep new` to create a proper project and replace the files where necessary. See [../package_managers.md#build2] for details.
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this file and add the documentation on how to build the project to package_managers.md similar to the other package managers like the following for Bazel:

??? example

    1. Create the following files:

        ```ini title="BUILD"
        --8<-- "integration/bazel/BUILD"
        ```

        ```ini title="WORKSPACE"
        --8<-- "integration/bazel/MODULE.bazel"
        ```

        ```cpp title="example.cpp"
        --8<-- "integration/bazel/example.cpp"
        ```

    2. Build and run:

        ```shell
        bazel build //:main
        bazel run //:main
        ```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the file and simplified the example to be closer to that example, but note that it's not working the same way for build2 as most of the other tools. I can still simplify it but at the cost of making the example over-simplified.

@nlohmann
Copy link
Owner

  • I can't seem to satisfy DCO, could you point me to my mistake? I did add the signature to the commits.

See https://github.com/nlohmann/json/pull/4587/checks?check_run_id=40451156048. There, a solution is described:

  1. Ensure you have a local copy of your branch by checking out the pull request locally via command line.
  2. In your local branch, run: git rebase HEAD~3 --signoff
  3. Force push your changes to overwrite the branch: git push --force-with-lease origin build2-usage
  • The static code analysis are applied on example code, is this normal? Should I ignore it?

Yes.

  • My code editor removed a bunch of trailing whitespaces automatically, we can see them in the changes. Should I revert that or is this ok?

Please revert.

@nlohmann nlohmann removed state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated please rebase Please rebase your branch to origin/develop labels Apr 13, 2025
Klaim added 4 commits April 13, 2025 17:52
Signed-off-by: Joël Lamotte <mjklaim@gmail.com>
Signed-off-by: Klaim (Joël Lamotte) <142265+Klaim@users.noreply.github.com>
Signed-off-by: Joël Lamotte <mjklaim@gmail.com>
Signed-off-by: Klaim (Joël Lamotte) <142265+Klaim@users.noreply.github.com>
Signed-off-by: Joël Lamotte <mjklaim@gmail.com>
Signed-off-by: Klaim (Joël Lamotte) <142265+Klaim@users.noreply.github.com>
Signed-off-by: Klaim (Joël Lamotte) <142265+Klaim@users.noreply.github.com>
Signed-off-by: Klaim (Joël Lamotte) <142265+Klaim@users.noreply.github.com>
@Klaim
Copy link
Contributor Author

Klaim commented Apr 13, 2025

I still have some cleanup to do, but no time left today, will continue soon. 👍🏽

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

Successfully merging this pull request may close these issues.

None yet

3 participants