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

Add support for modular build structure. #21

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

grafikrobot
Copy link
Member

This is part of the effort to make the Boost libraries "modular" for build and consumption. See https://lists.boost.org/Archives/boost/2024/01/255704.php and https://github.com/grafikrobot/boost-b2-modular/blob/b2-modular/README.adoc for more information.

This PR depends on the following other PRs being merged to both develop and master branches of the respective repos:

@pdimov
Copy link
Member

pdimov commented May 1, 2024

You'll have to walk me through these changes and explain why each is needed.

  • I don't think I ever plan to have a modular branch so I don't need the CI changes.
  • I don't like explicit [ this ] [ that ] ; and prefer the old school this ; explicit this ; that ; explicit that ;
  • The repetition of the DIST_DIR logic in every repo doesn't strike me as elegant. We should try to think of some way to factor it out.
  • Why are those all, dist and so on targets needed, and why does CI change from building tools/boostdep/build to tools/boostdep//dist?
  • Why are you adding link=static to my CI?
  • Why is it necessary to move the install rules from build/ to a root build.jam?

@grafikrobot
Copy link
Member Author

You'll have to walk me through these changes and explain why each is needed.

* I don't think I ever plan to have a `modular` branch so I don't need the CI changes.

As the PR is tied to the branch I'm developing in I need those changes. As otherwise there's no way for me to test that things are working as expected and without regressions. They are harmless though. :-)

* I don't like `explicit [ this ] [ that ] ;` and prefer the old school `this ; explicit this ; that ; explicit that ;`

Okay. I was being consistent with all the other changes in libs/tools I'm making. But I can change it to be old-skool.

* The repetition of the DIST_DIR logic in every repo doesn't strike me as elegant. We should try to think of some way to factor it out.

* Why are those `all`, `dist` and so on targets needed, and why does CI change from building `tools/boostdep/build` to `tools/boostdep//dist`?

To be consistent I'm adding an "all", and other targets. I still need to write up an explanation of those in README.md though.

* Why are you adding `link=static` to my CI?

I can probably remove that. It was just easier to debug building when I didn't have to worry about DLL linking at one point.

* Why is it necessary to move the install rules from build/ to a root build.jam?

To be notionally consistent with the project root declaring public targets. Hence equivalently to cmake.

@grafikrobot
Copy link
Member Author

Forgot to answer this one..

* The repetition of the DIST_DIR logic in every repo doesn't strike me as elegant. We should try to think of some way to factor it out.

It's not, but it works. But it's also now better than it used to be. As before it was done different for each of the tools. Ideally we should use the https://www.bfgroup.xyz/b2/manual/release/index.html#bbv2.builtin.features.install-prefix features. But I didn't want to go risking breaking things more than I had to.

@pdimov
Copy link
Member

pdimov commented May 1, 2024

If we copy-paste the same DISTDIR logic everywhere, it will eventually deviate and we'll be back to status quo.

@pdimov
Copy link
Member

pdimov commented May 1, 2024

As the PR is tied to the branch I'm developing in I need those changes.

You can keep these in your fork; I don't need them here.

@grafikrobot
Copy link
Member Author

As the PR is tied to the branch I'm developing in I need those changes.

You can keep these in your fork; I don't need them here.

I reworked the logic to avoid mentioning "modular" except for the single branch filter at the top. We can remove that single mention in a future change.

@grafikrobot
Copy link
Member Author

* I don't like `explicit [ this ] [ that ] ;` and prefer the old school `this ; explicit this ; that ; explicit that ;`

Okay. I was being consistent with all the other changes in libs/tools I'm making. But I can change it to be old-skool.

Did that.

@grafikrobot
Copy link
Member Author

* Why are you adding `link=static` to my CI?

I can probably remove that. It was just easier to debug building when I didn't have to worry about DLL linking at one point.

That's also done. Anything else?

This was referenced Jul 20, 2024
@pdimov
Copy link
Member

pdimov commented Oct 10, 2024

This still contains changes to ci.yml that I'm not willing to accept.

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.

2 participants