Added vcs-type subpackage.

[skasperski]

This allows to define a package that is located in a sub-directory of another package. Use cases are common multi-packages commonly used in the ROS world (one repository with many packages inside), but also cases like cmake-packages that come with their ruby/python bindings in ruby/python subpackages.

@planthaber @chhtz @doudou

[chhtz]

Nice!
I guess the rubocop complains should get resolved, though.

[g-arjones]

Isn't this going to be a problem for packages that have multiple subpackages? Since you are delegating the update and checkout calls, I believe you may end up with the same package being "checked out" or "updated" multiple times, potentially simultaneously.

[skasperski]

No, I did this exactly to avoid this issue that we had with our workaround for packages with many sub-packages so far. In this case the parent is only checked out and updated once.

[g-arjones]

Could you please elaborate? How does this prevent a race condition during parallel imports?

[skasperski]

It uses the standard way from autoproj for updating packages. When a package is required by 2 other packages, it is also updated only once, even though all downstream dependencies get updated.

Edit: Actually not so sure anymore that this is what happens. Gonna look into this some more...

[g-arjones]

I'm not sure I understand what you are saying... Suppose a simple workspace with three packages A, B, C, all three are directly selected; B and C depend on A.

The current behavior is: autoproj will call checkout on all 3 importers at the same time (that's fine since they have different internal states).

What you are implementing: autoproj will still call checkout on all 3 importers simultaneously (I don't see any change here that would modify this behavior), but now the importers of B and C will delegate the call to the importer of A, simultaneously, creating race conditions while accessing/modifying A importer's internal state.

Or am I missing something?

[chhtz]

Ideally, what we want is that the parent package is just something like a dependency of the sub-packages. But that does not work directly, since dependencies are only checked out after the package itself is checked out.

For reference, this is the current workaround used by simulation/mars/.*:
https://github.com/rock-simulation/simulation-package_set/blob/master/libs.autobuild
In the source the repositories are marked as "interactive" to prevent race conditions (but the repository is always updated multiple times):
https://github.com/rock-simulation/simulation-package_set/blob/master/source.yml#L12

[skasperski]

@chhtz @g-arjones I changed it to set the dependency to the parent package during checkout, which interestingly seems to work in all tests I did. So especially calling global amake with a non-existing repository containing 9 subpackages worked well. It now says "my/package/subpackage updated", which is technically a lie because the update will happen only later when "my/package updated" is reached, dunno how much of an problem that might be.

[skasperski]

I looking into how to solve this:

WARN: cannot snapshot simulation/gui_collection/cfg_manager_gui as the Autobuild::Subpackage importer does not support it

[g-arjones]

Autobuild cannot depend on autoproj

[g-arjones]

I don't think this is going to behave as expected, especially with --no-deps...

[g-arjones]

I think what you guys are trying to implement is feasible, but not as trivial to get right as we would want. The delegate pattern you were going for is promising, but you have to synchronize access to the parent's importer and make sure you don't check out or update the same package multiple times...

I would also suggest an API to create the importers so that the user doesn't have to define the importers manually.. Something like:

stack_package 'some_metapackage' do
    cmake_package 'package_a'
    python_package 'package_b'
end

[skasperski]

Yes, with --no-deps it will definitely not work, but I don't really see a way to fix that.

[skasperski]

I don't know who could implement such a more involved solution to this. I think the current version should work in most situations. Only he type subpackage was added, so it should not affect any existing workspace.

[planthaber]

We already talked about this, But it is not included (yet), because the other (sigle line) api ideas made it impossible/complicated/impractical to set defines etc. for the subpackages.

Your proposal actually was the latest result, as only this way we are able to add blocks to the subpackages.
Additionally it is directly clear what happens with the subpackage.

stack_package 'some_metapackage' do
    cmake_package 'package_a'
    python_package 'package_b'
end

For the Autoproj require: Do you think it woulf be better to place this in autoproj?: https://github.com/rock-core/autoproj/tree/master/lib/autoproj/autobuild_extensions