COVESA / vehicle_signal_specification

Vehicle Signal Specification - standardized way to describe automotive data
Mozilla Public License 2.0
307 stars 157 forks source link

Need for release-branches? #704

Closed erikbosch closed 4 months ago

erikbosch commented 5 months ago

This is a follow up to #646. There I requested information on whether someone needed branches of the type release/X.Y. No response was received and older "obsolete"/"stale" release/X.Y branches where we likely not will create any patch releases was recently removed.

Now it turned out that the branches actually were used by some downstream projects, causing builds to fail. By that reason I think it makes sense to revisit the decision. But if we agree that we should put the branches back, we need to agree and document what they shall contain, preferably in the wiki at https://github.com/COVESA/vehicle_signal_specification/wiki/Release-Instructions-and-Checklist

Older branches/releases

Future releases

g-scott-murray commented 5 months ago

I don't remember issue 646 ever being mentioned on a VSS call, but I did miss a few. The removal of the branches broke the AGL build for our in maintenance Pike release which uses 3.1.1, and Jan-Simon noticed that it also breaks LEDA due to them also referring to one (see https://github.com/eclipse-leda/meta-leda/blob/84edcd972ff4b465a8a3da88c43676e85cc49887/meta-leda-components/recipes-sdv/covesa/vss-release3_3.1.1.bb#L20). If branching is or has been done to create commits for releases, keeping those branches is IMO required, as downstream users expect them to be available due to the following chain of reasons:

  1. Upstream Yocto best practice is to not use tags in recipes, as they are not guaranteed to not change, and I believe result in bitbake having to check the source repository every time a build is done.
  2. As well, upstream Yocto has determined that referring to auto-generated archive files from github is unreliable, as github sometimes regenerates them, resulting in checksums changing and builds breaking. Referring to such files in recipes now generates a warning by default in newer Yocto releases. AFAICT currently the only available VSS release .zip or .tar.gz files available are auto-generated ones, none were ever explicitly published upon release to yield a stable location.
  3. (1) and (2) mean that fetching VSS in a bitbake recipe needs to be done by commit ID to have a reproducible build. Upstream Yocto recommends explicitly specifying the branch in this case. Hence, the removal of the already published branches breaking AGL and LEDA.

With respect to your questions, I would say:

erikbosch commented 5 months ago

Thanks for the feedback. For patch releases (X.Y.Z, Z>0), our current approach is that we do not create any branches until needed. That is a minor problem for us who have write rights (maintainers), as we easily could create a branch for preparing the patch when needed, but it makes it more difficult for other contributors as there is no branch they can use as target for pull requests. We have today as 4.X branch for content intended for next 4.X minor release, we could easily create a 4.0.X and 4.1.X branches as preparations for possible patches.

We could off course use branches release/4.0 and release/4.1 for that purpose as well. The important part then as I see it is just to document what the top of the release/4.1 branch is supposed to contain. Should it always be the last released 4.1.X version, or is it OK that the top is towards an unreleased (and thus only partially tested) commit or a release candidate like 4.1rc0?

My background for #646 is that I thought creating pull requests became more complex if you had a lot of stale branches where we do not expect any pull-request, so instead just keeping the branches that are relevant for pull requests would make it easier to select the relevant branch.

For releases, I was not aware that Github could regenerate the tar/gzip artifacts which they apparently do now and then, and that checksums then might be different even if tag and contents are the same. It would not be a problem for us to manually create tar/gzips for releaseas as well, we just need to agree on which that are needed and which command that shall be used to create them, then we can add it to the release process.

But if I should try to summarize your expectations and try to come up with a way of working, it seems to be like:

slawr commented 5 months ago

Upstream Yocto recommends explicitly specifying the branch in this case. Hence, the removal of the already published branches breaking AGL and LEDA.

Hi Scott (@g-scott-murray) , I don't remember that advice from my time working on Yocto. Can you provide a reference? I took a quick pass through the Yocto Development Manual, e.g. https://docs.yoctoproject.org/dev-manual/new-recipe.html#fetching-code and couldn't find it.

g-scott-murray commented 5 months ago

Upstream Yocto recommends explicitly specifying the branch in this case. Hence, the removal of the already published branches breaking AGL and LEDA.

Hi Scott (@g-scott-murray) , I don't remember that advice from my time working on Yocto. Can you provide a reference? I took a quick pass through the Yocto Development Manual, e.g. https://docs.yoctoproject.org/dev-manual/new-recipe.html#fetching-code and couldn't find it.

You are correct, I looked as well, and did not find anything definitive, so I perhaps overstated. However, it is the case that bitbake started emitting warnings unless a branch is specified in late 2021 (so with kirkstone / 4.0 and newer), which will push most people to follow what they see upstream doing with respect to specifying a branch. If you look in oe-core / poky, or meta-openembedded, the nobranch option is very rarely used. There are zero uses in actual recipes in oe-core AFAICS (there are a few references in tests), and only in 15 out of over 675 recipes that use git:// SRC_URIs in all of the layers in meta-openembedded by my grepping. This would suggest that while dangling commits/tags can indeed be handled by using nobranch with the bitbake fetcher, few projects actually do that.

g-scott-murray commented 5 months ago

@erikbosch I agree with your summation. In regards to your concern about documenting whether a release branch can contain commits as HEAD that are not tagged releases, I would lean towards documenting that that can be the case while work is in progress to create a patch release (i.e. while discussion takes place on whether certain fixes get picked or developed for it).

erikbosch commented 5 months ago

Tried to describe a possible branch/tag-strategy in #707. Feel free to take a look at it and comment if you have questions/opinions. My idea is not to merge the PR but rather to integrate it into into our release process in the wiki when we have agreed on way of working.

slawr commented 5 months ago

Upstream Yocto recommends explicitly specifying the branch in this case. Hence, the removal of the already published branches breaking AGL and LEDA.

Hi Scott (@g-scott-murray) , I don't remember that advice from my time working on Yocto. Can you provide a reference? I took a quick pass through the Yocto Development Manual, e.g. https://docs.yoctoproject.org/dev-manual/new-recipe.html#fetching-code and couldn't find it.

You are correct, I looked as well, and did not find anything definitive, so I perhaps overstated. However, it is the case that bitbake started emitting warnings unless a branch is specified in late 2021 (so with kirkstone / 4.0 and newer), which will push most people to follow what they see upstream doing with respect to specifying a branch. If you look in oe-core / poky, or meta-openembedded, the nobranch option is very rarely used. There are zero uses in actual recipes in oe-core AFAICS (there are a few references in tests), and only in 15 out of over 675 recipes that use git:// SRC_URIs in all of the layers in meta-openembedded by my grepping. This would suggest that while dangling commits/tags can indeed be handled by using nobranch with the bitbake fetcher, few projects actually do that.

A belated thanks for the confirmation. I had two reasons to ask in mind. First, was that whilst I had done a lot of development with Yocto previously (BSPs, meta-ivi, GDP etc), not so much more recently and I wondered if bitbake or Yocto best practise had changed. Second, was to simplify the release strategy discussion if Yocto had no specific demand on an upstream beyond obvious good engineering practice. It seems that is the case unless I miss something?

I agree that ideally downstream builds should not be broken by changes in release artifacts. Moving tags being an obvious one. Long lived projects should also be able to evolve other time and if that evolution is going to be disruptive to downstreams we should mitigate where possible. In the case of the release branch deletion I raised the point that changes such as that should be listed in the release note. For people not directly contributing to the project the release note is a natural source of migration information. In a case like this where it is housekeeping that is not time dependent, it could be marked for deprecation as we currently do for code to give people a release cycle to adopt to the change. That would be one suggestion I would make for the review.

erikbosch commented 5 months ago

I agree that ideally downstream builds should not be broken by changes in release artifacts. Moving tags being an obvious one. Long lived projects should also be able to evolve other time and if that evolution is going to be disruptive to downstreams we should mitigate where possible. In the case of the release branch deletion I raised the point that changes such as that should be listed in the release note. For people not directly contributing to the project the release note is a natural source of migration information. In a case like this where it is housekeeping that is not time dependent, it could be marked for deprecation as we currently do for code to give people a release cycle to adopt to the change. That would be one suggestion I would make for the review.

Here we need to be clear on what we mean with "release note". As of today the release note is what we create when we create a release, like this is the 4.1 release note, and they have historically focused on changes in code compared to previous version. So if we for instance hypothetically would agree and decide to delete the "release/2.0" branch today, in which release notes should it then be reported? Shall we update the 2.0 release notes, should it be listed in the release note of last release (4.1) or should it be mentioned first in the release notes of the next release?

Another possibility would be the CHANGELOG file which we could update at any time, but that one is also a bit cumbersome to maintain now when we work in multiple branches. Potentially a page in the wiki would be better to show status of various releases, like end-of-life dates and so on.

slawr commented 5 months ago

I agree that ideally downstream builds should not be broken by changes in release artifacts. Moving tags being an obvious one. Long lived projects should also be able to evolve other time and if that evolution is going to be disruptive to downstreams we should mitigate where possible. In the case of the release branch deletion I raised the point that changes such as that should be listed in the release note. For people not directly contributing to the project the release note is a natural source of migration information. In a case like this where it is housekeeping that is not time dependent, it could be marked for deprecation as we currently do for code to give people a release cycle to adopt to the change. That would be one suggestion I would make for the review.

Here we need to be clear on what we mean with "release note". As of today the release note is what we create when we create a release, like this is the 4.1 release note, and they have historically focused on changes in code compared to previous version. So if we for instance hypothetically would agree and decide to delete the "release/2.0" branch today, in which release notes should it then be reported? Shall we update the 2.0 release notes, should it be listed in the release note of last release (4.1) or should it be mentioned first in the release notes of the next release?

Another possibility would be the CHANGELOG file which we could update at any time, but that one is also a bit cumbersome to maintain now when we work in multiple branches. Potentially a page in the wiki would be better to show status of various releases, like end-of-life dates and so on.

Hi Erik, by "release note" I meant the logical release note artifact(s), i.e. without being specific, but you are right details matter for the actual process. For the specific example you give, assuming we can time the change, I was thinking the VSS vspec depreciation process could be a candidate. So the release note for release 4.1 might state "xyz is marked for deletion/change in the next minor/major release (delete as appropriate). Then the 5.0 release note say, states "xyz is deleted". The first flags the upcoming change (downstreams can prepare), the second explains possible cause if the downstream missed the note or didn't prepare and came wondering why their build just broke in 5.0. In both cases the downstream is given the tools to adapt. That could be done for any major change, such as git URL migration. Just an idea.

As to the detail of where etc. then I don't have strong opinion. Whatever makes sense to the people like yourself making the releases and who likely have the best handle on the nuances. So if CHANGELOG has more information related to migration then there is fine. The existence of the log just needs to be prominent in the release artifacts, i.e. easily discoverable by downstream.

slawr commented 4 months ago

Hi, I just wanted to add that my comments here are limited to the process aspects of communicating changes. I am not implying I am for/against the removal of release branches.

erikbosch commented 4 months ago

CLosed according to https://wiki.covesa.global/pages/editpage.action?pageId=48529442 2024-02-13