-
Notifications
You must be signed in to change notification settings - Fork 44
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
bob managed layers #561
base: master
Are you sure you want to change the base?
bob managed layers #561
Conversation
Taking a step back: what is the actual motivation of having Bob managing the layers? What is the benefit of adding this feature compared to git submodules? I think we should have a compelling benefit to add the complexity. Could you shed some more light on this? If we go this route, the following things come to my mind:
|
There are two reasons: My current layer structure would look like this if it would be supported:
IMO this can't be solved with the current layers except by splitting the recipes much more. Or do you have any suggestions how this could be solved? The other reason is "because some developers don't like submodules." We have (at least) one project where the layer content has been copied into the recipes just to avoid the need of using submodules. (I'm not sure if using bob managed layers would help them...)
I guess we can also add a policy for this?
Yes, the loop is missing.
I also thought about this as it would avoid the need of adding a new command. Not sure what would be more convenient. |
Right. This makes sense and I also cannot see how it could be solved with git submodules either. So let's do it.
I'm not sure either. But having an "automatic" layers update that behaves like the SCMs in regular recipes might be more attractive.
No, I wouldn't unless somebody speaks up. I have never heard of anybody using nested layers. We can still add such a policy if somebody comes around.
I think we need both. The layers command makes perfectly sense. I was thinking about the case where somebody clones a project and expects it to "just work". So
should just work, even when layers are used. On the other hand, you might not always want the automatic layer pulling. So some overridable option to bob build/dev makes sense. |
We have similar use cases as reported by @rhubert. I often get the question why the layers can't be managed better via bob (in analogy to the SCMs in the recipes). The current situation is that when a layer is updated, the whole team must be notified about this to update their local gitsubmodule(s). Sometimes it is forgotten and the colleague work on an older version. It would be nice if this update could be done via However, if the main recipes have a breaking dependency on one or more layers, these would also have to be updated. In this use-case, updating the layer alone would lead to problems and you would have to explicitly make a pull on the recipes (if you know it, which is mostly not the case). The use of nested layers also makes sense (and we are now at the point where we absolutely need it). I have already experimented with nested layers but encountered similar problems to those reported by @rhubert. As mentioned:
|
I think the last point would become valid if |
I had the following scenario in mind (assuming the layers are no longer managed via gitsubmodules): layers:
- foo:
scm: git
url: git@foo:/foo.git
branch: master # <-- layer is floating Changes are made to the layer that also affect the main recipes (e.g. incompatible changes). Developer 1 adjusts the main recipes accordingly and push the changes to the remote. Developer 2 or a whole team knows nothing about this and would automatically update the layer with the next But if I think more about it for a while, it makes no sense to work with floating layers. If several layers are nested inside each other, this increases the level of complexity considerably, becomes confusing and is error prone. I think it would be better to always hard link the layers to a tag/commit like the gitsubmodules already do it. So I agree that this was not the best idea. ;-) Nevertheless, it would be a nice feature when the main recipes are on a branch and there are new changes on it you can you can get a notification from bob about this. In our team, at least, this is a recurring use case where you always have to make sure that everyone is really synchronized. I think this can be solved more elegant (it should be relatively easy for Bob to provide something like this?) |
I think this can't be easily done as bob has no knowledge about where the recipes are coming from. Could be As of today you can add a https://bob-build-tool.readthedocs.io/en/latest/manual/configuration.html#hooks Note: you can not simply |
I pushed some updates to the code to avoid the internal package.. It's still not ready for review... Anyway - after some internal discussions we think that merging the (sub-) layers of different layers by their name/url/whatever might not work very well. Instead we want to filter the layers provided by a sublayer when depending on it: Given a layer foo with # foo - config.yaml
layers:
- bar:
[...]
- baz:
[...] The user of this layer can avoid the use of layers:
- foo:
checkoutSCM:
scm: git
url: ....
commit: ...
useSubLayers: [baz]
- bar:
[...] Not adding |
I agree that merging them on the basis of the URL is certainly not viable. But why not by name? Projects/layers that include other layers should know the naming below. Only if two layers refer to some common third layer by a different name things go south. But is this really likely to happen? One could imagine to encourage people to use reverse polish notation for the layers (e.g.
I'm not sure that this is better. The behaviour can get very subtle I guess. Also, how it it supposed to work transitively (control the sub-sub-layers)? I don't have any better idea at the moment but if feels its more like hack... |
The idea was to have more control about the layers. e.g. if a common sublayer is used by 2 different layers one might need to use the newer layer spec. And if there are two common sublayers by 2 different layers they might cross reference newer versions of their sublayers:
To make this work we'd need to select |
My current line of thinking goes like this: We would always flatten the layer dependency tree by name. This happens even for layers that are not SCM-managed but just referenced in The project has the highest precedence. So if a layer "b" is defined in the project, it would override any deeper layer dependencies (layer While this can still be very subtle what is actually used, it at least is consistent with the current layer precedence. For SCM managed layers, Bob should certainly not even checkout layers that were already satisfied by higher precedence layers. So it will be more or less obvious what is going to be used. Does that make sense? |
e17e94b
to
1aa7863
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #561 +/- ##
==========================================
+ Coverage 88.57% 88.74% +0.17%
==========================================
Files 46 48 +2
Lines 14725 15082 +357
==========================================
+ Hits 13042 13385 +343
- Misses 1683 1697 +14 ☔ View full report in Codecov by Sentry. |
43d30d5
to
59043e2
Compare
I think this is now in a state where it might make sense to start reviewing process... One note to the last commit XXXXXX: DevelopDirOracle: make a singleton + fix recalculation. This tries to fix the failing 62 recipes.setUpdateLayers(not args.execute_buildonly) in |
Thanks. I'll have a look in the next days. |
I'm not entirely sure if all of the following makes sense but these are my first thoughts after skimming though the stack for an hour or so.
The layer schema looks more complicated that it needs to be. How about the following? layers:
- name: foo
scm: git
url: ...
- name: bar
... I'm not sure about the I was hoping that we could integrate the layers update in the regular build TUI. Especially to use multiple jobs in parallel. But that may be something for a later step. |
Thanks for the comments :)
The layers update is also needed for non build commands like
👍 will do this.
👍
Thanks - I wasn't aware of this ...
I think this is a reasonable simplification. We'd loos some flexibility but I can't think of a use-case for having multiple git-scms for a single layer or something like this..
The layers update is usually fast (compared to build + parse times). Not sure if there is a benefit of doing this in parallel, but I'll keep it in mind. |
I'm not sure that this is a good idea. Especially |
bb0e067
to
af27517
Compare
Allow to specify scm properties for layers and use them to checkout / update the layers. Only git, svn, cvs or url scms are supported. Having layers with a SCMSpec they are only updated during bob dev|build if --build-only is not set. Otherwise they are not touched by any existing bob command. In addition the new `bob-layers` command can be used to update layers and show their status the same way like `bob status` does.
I did some rework and changed the code following your suggestions. Take your time for review I'm N/A the next weeks, so I there will be no more update from my side before |
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 think this is heading in the right direction. I still miss the loop to handle arbitrarily nested layers. Did I overlook it?
|
I'm looking into the PR in detail but it will take some time. I'll tinker with it and probably push some fixups if that's ok... |
I'm not really comfortable with the default.yaml handling during the layers update so far. It's also why the tests are failing. We try to parse the user configuration files but paper over any problem. This will certainly lead to surprising results. Maybe we should not even try that? Or do we need variable substitution in the layers SCMs? |
I think having If this won't work I think no support for Overrides+defaults in the layer scm's would be also acceptable. In this case one could still name |
I see. In principle I fully support to have scmOverrides and environment substitution. The problem is that this creates us a circular dependency. To fetch and/or update the layers we have to naturally go in top-to-bottom order. Even though it may not evident, the parsing of I'm not sure how we can solve this elegantly (or at all)... |
Thanks for the explanation...sometimes it takes longer but now I got the point...I guess.... I removed the parsing of the |
This is a very rough first draft of support for layers handled by bob to get some early comments about this.
The idea is to add a scm-spec to the
layers
section of theconfig.yaml
:A layer specified like this also changes the behavior when using nested layers where multiple layers depend on the same layer as the layer structure is flattened and each layer is checked out only once. This make it possible to have the same recipes either used as root-recipes repo or as layer from another recipes package. (ATM this is done by matching the name of the layer only, maybe the url should be used instead or a additional
uuid
,... ??)ATM a internal
.bob_layers
root package is generated for this with dependencies to a package for each layer. I don't know if this is a good idea but it enables reusing most of the code. ;)If this is a viable approach it would need some filtering for the other commands like
bob ls
to avoid the.bob_layers
package is shown there as well. Maybe this filtering can be done by conditionally adding the.bob_layers
to thevirtualRoot
and providing the define by the layers-command? But this would result in longer parsing times when switching frombob layers
to any other bob command.