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

Git manifest provider support for one repo multiple packages #127

Closed
wants to merge 1 commit into from
Closed

Git manifest provider support for one repo multiple packages #127

wants to merge 1 commit into from

Conversation

veimox
Copy link
Contributor

@veimox veimox commented Jan 14, 2019

Adds supports for packages like ros-navigation where there are multiple packages in one repo. In line with PR #126, there is a use case where the source repository can also be the release repository. This implies getting the manifest from a subfolder rather than from the root.

The PR sets the default behavior as of today and in case of failure will check in the subfolder. If not, will throw the exception as expected.

Adds supports for packages like [ros-navigation](https://github.com/ros-planning/navigation) where there are multiple packages in one repo. In line with PR #126, there is a use case where the source repository can also be the release repository. This implies getting the manifest from a subfolder rather than from the root. Notice this is not a problem if the release repository (as designed by bloom) collects every source subfolder into a branch for that single package.

The PR sets the default behavior as of today and in case of failure will check in the subfolder. If not, will throw the exception as expected.
@dirk-thomas
Copy link
Member

The problem with this patch I see is that it makes assumptions about the structure within the repo. While that might work for the ros-navigation repo I don't think it will hold generically.

@dirk-thomas
Copy link
Member

@veimox Are you planning to iterate on this patch or should it be closed?

@veimox
Copy link
Contributor Author

veimox commented Jan 26, 2019

Sorry for the delay. I see your concern here so i'm gonna try to synthesize the options.

The problem here lays on the functionality of this tool to generate a cache distribution, needed for the reverse dependency search of a package. The cache is generated from the package xml's of a package that can be alone sited on a repo/branch being the file at the root. That premise works because in the ros buildfarm model, there is a release repo where there is an enforcement of 1-1 repo/branch per package being always the case in which the package.xml is found at root. This is not an enforcement for source repos.

My conflict with this tool is there because it turns out that we use the source repo as release repo and, because we don't have this enforcement, the cache generator has to "search" for the package.xml. It is true that in our use case (based on a sort of tacit convention) some packages are contained within the same repo in a single subdirectory fashion like ros navigation. I see three options regarding finding the package.xml:

  1. As of today, enforce that the file has to be in the root of repo/branch. Then this PR can be closed.
  2. Search for the right file in a "complete" manner.
  3. Search for the file based on some limited cases.

The proposal of this PR is a single case of 3 implemented only for git-cloned repos.

This tool should IMHO not be tight to the ros-buildfarm conventions and as such I think the right solution should be 2 or 3. I don't like 2 that much becase of the performance hit that would imply (is just a guess) so I propose to extend this PR by two things:

  1. increasing the set of cases to a walking search from 1 to up to 4 folders.
  2. implementing this for github repos too.

@dirk-thomas
Copy link
Member

I am not sure what the best solution is but the goals are:

  • Any change should not inflict a performance hit on existing release entries pointing to actual gbp repos.
  • Any change should work for arbitrary layouts in the source repositories.
  1. Search for the right file in a "complete" manner.
    I don't like 2 that much becase of the performance hit that would imply

This is indeed a problem. You can easily see this by comparing the time it takes to create a cache for the released repos (one http request per package) vs. create a cache for the source repos (clone each repository and then crawl the local working copies).

increasing the set of cases to a walking search from 1 to up to 4 folders.

If the package are not in direct subdirectories named after the package you don't have a way to guess their location. As a consequence you will need to obtain the directory structure of the repo (either in full or incrementally) which is both fairly expensive.

An alternative I could imagine would be to choose between the two already available approaches based on the version number of the release. If it includes a Debian increment keep the current behavior. If it doesn't fallback to a full clone and crawl (like for source entries). That will make your use case not faster than the generation of source cache but doesn't affect the release caches generated for the ROS distributions.

@mikepurvis
Copy link
Contributor

@veimox I tried to pitch essentially this option years ago and was (rightly) rebuffed on the grounds that only a bloom-generated GBP repo belongs in the release stanza— see the discussion in #65. The outcome of that discussion was a bunch of work that went into making python-rosdistro able to generate source manifest caches, which are consumable by python-rosinstall-generator. Although they're not documented well, these features do exist and have been relied on by Clearpath and other companies for years.

Anyway, I already explained this in the #126 discussion, including the exact commands you'd need to generate and use the source cache. Please give it a try; you'll find it much more straightforward than trying to shove extra functionality into the release caching mechanism.

@veimox
Copy link
Contributor Author

veimox commented Jan 28, 2019

@mikepurvis I didn't understand well your comment in that PR but now I do. Thanks!

In a sense I think this tool should be more decoupled from bloom and this PR could be the one of many to achieve that but should not be merged by itself.

My motivation here was to create a superbundle and I see two ways for doing so without modifying existing tools:

  1. The one proposed by Mike to create it from sources. This is: create a rosdistro with the desired tags, use that file along with python-rosdistro and others to generate the dependency tree and a .rosinstall with all packages. Fetch repos, create deb control files and compile super bundle. This options allows to recreate the same file and takes 30-60 minutes depending on your needs and distro.
  2. The one we are currently going with that is creating it from the released deb files. Due to the well isolated file structure of ROS (everything into one folder) combining deb files with some debian-kunfu is quite straight forward. What I'm doing right now is creating the dependency tree (from the deb metadata, not rosdistro), downloading those files, extracting them into the same folder and the turning that folder into a new deb file. This creates a super bundle out of released binaries in a (1) more deterministic -packages are guaranteed to build and have passed tests- and (2) faster -5 minutes in ci- way. The problem with this option though is that lack of selecting which versions you want in the superbundle because as of today the ROS servers only keep the latest, but that is for another discussion. This option only plays well for private companies if they have a private APT repo.

All in all, I think this PR can be closed. I let @dirk-thomas to do it in case there are new comments.

@dirk-thomas
Copy link
Member

As mentioned already before I am fine to extend the supported use cases to enable using a tagged source repo in the release section if the logic to build the release cache is extended with the two goals stated.

If you don't want to continue on this route I would suggest to revert #126 though since on its own it isn't valuable.

@mikepurvis
Copy link
Contributor

Yeah, we considered the "combine debs later" option and abandoned it largely for the reason you give— too much bookkeeping needed to manage which versions went where (particularly with the buildfarm's constant rebuilding, where the deb version numbers all contain these mostly-meaningless timestamps), and it didn't feel like a great option for maintaining multiple threads of development, such as needing to bump the version of some package on a bundle releases three months ago, particularly if the bugfix needs to break ABI and trigger more downstream compilations.

We also really wanted a clear road to bypassing much of the release/bloom process and being able to generate binaries from devel branches, feature branches, etc, but without having to contemplate some kind of auto-blooming scheme.

Anyway, we've attacked the compilation time issue with aggressive caching of built products, which is possible using the --isolate-install option of catkin_tools, but that obviously has a tooling cost too, so nothing is really free. We also just throw hardware at the builds, as they're highly parallel.

@dirk-thomas
Copy link
Member

We also really wanted a clear road to bypassing much of the release/bloom process and being able to generate binaries from devel branches, feature branches, etc, but without having to contemplate some kind of auto-blooming scheme.

And I think that is exactly where the proposed case is better. Using devel branches is already possible using the source entries but relying on actually tagged versions of the source repo is certainly more stable.

With a tag from a source repo listed in the release section (without any involvement of bloom) it becomes easy to build a workspace with all tagged versions and you can also easily level them on demand. ANd that would still be beside the option to use devel branches when necessary.

So in general I like the described use case.

@mikepurvis
Copy link
Contributor

mikepurvis commented Jan 28, 2019

That's fair. I know we've talked here in the past about a new attribute for representing "latest tag" within the source stanza (see #92), but nothing much has come of it— the workflow of just changing the source/version attribute to the tag name and then tagging the overall rosdistro repo has worked fine for us.

If the release stanza can be made more useful in that regard, that'd be great! Currently we don't use it at all.

@veimox
Copy link
Contributor Author

veimox commented Jan 29, 2019

I have gone through all the issues presented by @mikepurvis and similar and it seems I am trying to do the exact same as he has already done. This PR and his are the exact same.

After thinking for a while the source option (option 3 as in here) I think is the way to go and does not require any contribution in this package. Still the PR #126 is not wrong because it follows the Debian convention so I would not revert it. Im prone to close this one though.

Thinking in a way possible way forward, I think the ros-infrastructure organization should have an answer for those that want to do point releases of a whole robot software (point release, super bundle or whatever we wanna name it). Here the discussion is upon the from sources solution although there are more like from binaries , the one proposed by colcon (used by AWS), Docker, snaps... Either documenting all or just one is not up to me but I can definitely put hours on this task if the decision is taken.

@dirk-thomas
Copy link
Member

Still the PR #126 is not wrong because it follows the Debian convention

I don't think the Debian convention is relevant in this case. When using a release repo in the release section the version must have a Debian increment. So if we drop the idea to support source repos in that section there is no need to relax that check. Hence I will revert #126 if we decide to close this ticket.

@dirk-thomas
Copy link
Member

I think the ros-infrastructure organization should have an answer for those that want to do point releases of a whole robot software (point release, super bundle or whatever we wanna name it).

I think there are already different approaches in use and it is mostly a matter of someone taking the effort of writing them down.

I still think supporting source repos with only a tagged version in the release section would be a good case to cover. It would allow to clearly describe sets of released packages in parallel to referening their development branches.

@veimox If you are not interested in this anymore and will use one of the other mentioned approaches please go ahead and close this ticket.

@veimox veimox closed this Mar 4, 2019
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.

4 participants