-
-
Notifications
You must be signed in to change notification settings - Fork 234
Feature/project search #304
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do you really think this complication worth it? I would guess that in the most user cases all the projects under specified paths will be loaded anyway, and for a package manager like vcpkg it is not a big deal to customize/optimize search scope via
project-searchrule.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.
Not sure I follow that. The use case I'm working on ATM is the case of a modular Boost. Where definitely not all projects would be loaded even though they could be under the same specified path. But maybe I'm not understanding what you are thinking of.
Recently I've come to learn that everything is a big deal in vcpkg because it actually does very little for you :-)
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 project (Boost) that has Jamroot will eventually include everything in its subdirectories
Can't Boost superproject (Jamroot) have that loader which will load a needed subproject when you request a library? Or your vision is that every Boost library will get its own Jamroot and libraries will no longer will be rooted to the superproject? But then I don't understand who and how will set and update these mappings for every library.
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.
But it doesn't have to.
A project that has just a top-level Jamfile automatically also works as if that Jamfile was the Jamroot. That's been around for some time. The goal is two fold.. 1. support the use case, for Boost, where there is a super-project Jamroot and 2. support the use case, for Boost, where the super-project doesn't exist (or there is no Jamroot/Jamfile at that root.
But perhaps a concrete example is better. Here's the modular Boost I've been working on https://github.com/grafikrobot/boost-b2-modular. As it currently stands there the following are possible:
project boost .. ;declaration withproject-search /boost : libs ;and everything will still work.cd /*/boosroot/libs/serialization/build && b2 --project-search=/boost:/*/boostroot/libs ...and it will build serialization without problems. (* = absolute or relative path).A package manager, or equivalent, would be doing (3) if they want modular Boost packages. Although they would probably do a bunch of individual key:value mappings for each. And they could also do (1), or (2), for monolithic builds (probably .. as I'm still working out all the details on that).
Does that help answer the questions? Or is it creating more questions? ;-)
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.
From my understanding of the written here is that this will only work because boost superproject loads serialization and all its dependencies, am I wrong?
On the one hand it is expected from package managers to know all the dependencies of the requested library, on the other hand the current solution does not provide a way to discover dependency tree to construct such b2 invocation. vcpkg struggled with that quite a bunch and currently solves the issue with their own header scanning script + boostdep mappings.
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.
You are wrong :-) The way it will works in modular Boost is that individual libraries will need to specify their dependencies in Jamfiles. The method is the usual one of just having a target reference. For example serialization/build.jam (aka jamfile) would look like this https://github.com/grafikrobot/boost-b2-modular/blob/b2-modular/patch/libs/serialization/build.jam#L8. And so one for dependencies recursively. With this PR B2 when it tries to resolve those targets would split them into the
/project//targetparts and then use the user specified mappings to find the, for example,array/build.jamand do ause-porject /boost/array : .../array ;And then look up theboost_arraytarget. Hence no need or involvement from a super-project.But it could provide such information since it's going to be present in the build of the library itself. I.e. we could add an introspection feature to B2 that, given a target, spits out the list of external project dependencies (which would be the other Boost libs in this case).
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.
I see what I have missed:
libspart in--project-search=/boost:/*/boostroot/libs, which looks so strange and alien while still not working for sublibs and stuff intoolsfolder.I'm sorry to say that but I have to -- it looks ugly, writing the same stuff twice... Is it impossible to make
<library>/boost/arraywork? Even better if it could be just simple/boost/array. It probably could be achieved by considering project-id reference as implying adding every project top-level target as<source>.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.
Well.. it's something entirely new. So some strangeness is expected. I thought about a couple different syntax options on the CLI and ENV. But this was the least problematic given all the constraints of the multiple platforms.
Maybe. It would mean finding a way for a project declaring some target as the one to add to the requirements and usage-requirements. Which would be used if a bare project reference is asked for.
Maybe. The easier way would be to add some other init argument to define those project references.
So certainly some things to think about. But those are orthogonal to this PR. As they would be additions to synthesize the references. I.e. things for some future PRs.