search

OSSystems / meta-browser

OpenEmbedded/Yocto BSP layer for Web Browsers
MIT License
183 stars 189 forks source link

chromium: Fix patch enumeration #739

Closed MaxIhlenfeldt closed 9 months ago

MaxIhlenfeldt commented 1 year ago

Missed in #738 that we already have 0031-Fix-ARM-build-with-recent-glibc.patch.

MaxIhlenfeldt commented 1 year ago

Also, this PR maybe isn't the perfect place for it, but as it's already about patch numbers: do we just keep increasing the patch numbers forever, even as we remove some patches over time? There are quite a few holes in our numbering already.

rakuco commented 1 year ago

do we just keep increasing the patch numbers forever, even as we remove some patches over time? There are quite a few holes in our numbering already

When I worked on recipe updates on my own, I think I ended up prefixing everything with "0001" because I was doing it manually (except if I was backporting a patchset with multiple patches). If you're using devtool I think it's done automatically, otherwise I personally don't mind what the patch number is. Not sure if there are official guidelines from OE though?

MaxIhlenfeldt commented 1 year ago

If you're using devtool I think it's done automatically

I tried getting it to work, but wasn't able to. I currently just do it by hand.

Not sure if there are official guidelines from OE though?

I couldn't find any in a quick search, and looking at some meta-oe recipes, it seems like there aren't any re: numbering. I've seen recipes that don't have numbers at all, some have multiple different patches with the same number, some have a mix of numbered and un-numbered patches.

I quite like the numbering, so I'd propose we always ensure the numbers increase by one (i.e. no holes). To avoid frequent renaming, it might make sense to have a separate naming scheme for backports, I think just omitting the number for backports would be fine. Wdyt?

rakuco commented 1 year ago

I quite like the numbering, so I'd propose we always ensure the numbers increase by one (i.e. no holes). To avoid frequent renaming, it might make sense to have a separate naming scheme for backports, I think just omitting the number for backports would be fine. Wdyt?

In the past I used to do the opposite: use numbers for the backports (simply because it was easier to use git format-patch on an upstream checkout and copy the patch file here without having to rename it) and not use numbers for things that remain, like 0003-v8-qemu-wrapper.patch.

Note that either workflow will probably break if someone decides to use devtool in the future -- see commit 98b68bae8f777107509fd2d8143c9cc9c831b57c which renamed several patches, for example.

kraj commented 10 months ago

I think on major version upgrades it can be renumbered if some get dropped. If you use devtool workflow this should happen automagically.

MaxIhlenfeldt commented 10 months ago

I think on major version upgrades it can be renumbered if some get dropped. If you use devtool workflow this should happen automagically.

I'd forgotten about this, thanks for reminding me about it. As mentioned above, I had some problems using devtool, so in the end I just wrote my own script that does everything (except resolving conflicts, of course).

My personal preference would be to keep the numbering for non-backport patches (always increasing by one), and name the backports "Backport-XXX.patch", so that we don't have to frequently re-number the non-backports.

Let me know if you think that's a bad idea; if not, I'll update this PR to apply the new scheme once #757 has been merged.

kraj commented 10 months ago

Devtool drops backports if they are straight forward automatically as well but prefixing the name is fine it makes it readable

MaxIhlenfeldt commented 9 months ago

@rakuco @kraj please let me know if you agree with the re-organization I've applied in this PR's latest commit.

MaxIhlenfeldt commented 9 months ago

How does it go with devtool ? I think it will be cool to get it friendly to devtool as well while you are here

I'm not very familiar with devtool, and the times I've tried to figure out the update workflow for Chromium it didn't work... but if you can share the needed devtool steps, I'm happy to make sure it works well with the re-organized patches.

kraj commented 9 months ago

How does it go with devtool ? I think it will be cool to get it friendly to devtool as well while you are here

I'm not very familiar with devtool, and the times I've tried to figure out the update workflow for Chromium it didn't work... but if you can share the needed devtool steps, I'm happy to make sure it works well with the re-organized patches.

devtool modify chromium-x11
devtool build chromium-x11
MaxIhlenfeldt commented 9 months ago

Thanks! However, devtool modify chromium-x11 already gives me an error. I don't think it's worth spending more time trying to get this to work, as I already have a functional workflow. If this PR makes devtool harder to use, we can just fix it in a follow-up. Thus, I'll go ahead and merge this, so I can start working on the 118 update.

kraj commented 9 months ago

Thanks! However, devtool modify chromium-x11 already gives me an error. I don't think it's worth spending more time trying to get this to work, as I already have a functional workflow. If this PR makes devtool harder to use, we can just fix it in a follow-up. Thus, I'll go ahead and merge this, so I can start working on the 118 update.

Yeah that’s fine I think @shr-project had it working iirc in past