| Message ID | 20230914161828.3662-2-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | net/lwip: add lwip library for the network stack | expand |
Hi Maxim, On Thu, 14 Sept 2023 at 10:20, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > add external lwIP library as a git submodule. Oh dear...what is the motivation for using a submodule? Our current stack is nicely integrated into U-Boot. This would make moving between development branches much more painful. I would much prefer that we bring in the necessary code, and that you send a patch every 3 months or so to deal with updates, making sure there are no code-size regressions. Regards, Simon
On Thu, 21 Sept 2023 at 07:06, Simon Glass <sjg@google.com> wrote: > Hi Maxim, > > On Thu, 14 Sept 2023 at 10:20, Maxim Uvarov <maxim.uvarov@linaro.org> > wrote: > > > > add external lwIP library as a git submodule. > > Oh dear...what is the motivation for using a submodule? > > Our current stack is nicely integrated into U-Boot. This would make > moving between development branches much more painful. > > I would much prefer that we bring in the necessary code, and that you > send a patch every 3 months or so to deal with updates, making sure > there are no code-size regressions. > > Regards, > Simon > I would like the project maintainer to make the final decision. And this time I'm trying to understand how lwIP maintenance works. And how long does it take to merge a patch to lwip. For the latest Ilias comment I did a fix to lwip, which is pending. https://lists.nongnu.org/archive/html/lwip-devel/2023-09/msg00004.html and created a bug with the same patch: https://savannah.nongnu.org/bugs/?64697 And it's interesting when patches get merged. Also there is a long list of not yet accepted patches (86 open items): https://savannah.nongnu.org/patch/?group=lwip I am afraid that if lwip patch acceptance will be too slow it also can slow down U-Boot development. Best regards, Maxim.
On Wed, Sep 20, 2023 at 07:03:07PM -0600, Simon Glass wrote: > Hi Maxim, > > On Thu, 14 Sept 2023 at 10:20, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > > > add external lwIP library as a git submodule. > > Oh dear...what is the motivation for using a submodule? That we don't have a better alternative. Using "git subtree" has it's own problems and won't make things overall better. > Our current stack is nicely integrated into U-Boot. This would make > moving between development branches much more painful. It really shouldn't matter in that case. Unless you're trying out a new lwip upstream commit, nothing changes in there. It _may_ mean that if we want to update lwip it's not something that can be staged first in the -next branch but instead something to pull in just after release, but I'd need to see how smart or not git is today about things like that. > I would much prefer that we bring in the necessary code, and that you > send a patch every 3 months or so to deal with updates, making sure > there are no code-size regressions. And I much prefer something that will make sure that we don't start making changes in upstream code and diverging. I don't think there's a mechanic short of submodule that will do that for us.
On 21.09.2023 09:09, Maxim Uvarov wrote: > On Thu, 21 Sept 2023 at 07:06, Simon Glass <sjg@google.com> wrote: > >> Hi Maxim, >> >> On Thu, 14 Sept 2023 at 10:20, Maxim Uvarov <maxim.uvarov@linaro.org> >> wrote: >>> >>> add external lwIP library as a git submodule. >> >> Oh dear...what is the motivation for using a submodule? >> >> Our current stack is nicely integrated into U-Boot. This would make >> moving between development branches much more painful. >> >> I would much prefer that we bring in the necessary code, and that you >> send a patch every 3 months or so to deal with updates, making sure >> there are no code-size regressions. >> >> Regards, >> Simon >> > > I would like the project maintainer to make the final decision. > > And this time I'm trying to understand how lwIP maintenance works. And how > long does it > take to merge a patch to lwip. For the latest Ilias comment I did a fix to > lwip, which is pending. > https://lists.nongnu.org/archive/html/lwip-devel/2023-09/msg00004.html > and created a bug with the same patch: > https://savannah.nongnu.org/bugs/?64697 > And it's interesting when patches get merged. > > Also there is a long list of not yet accepted patches (86 open items): > https://savannah.nongnu.org/patch/?group=lwip The list of open bugs and patches has largely to do with users sending things in the form of "this or that doesn't work for me, here's my poor quality patch that fixes exactly my issue". We simply don't have the manpower to check all that for correctness and for not breaking other use cases. Nearly noone sends a working test case for things. But we're trying to catch up. > I am afraid that if lwip patch acceptance will be too slow it also can slow > down U-Boot development. Our response time greatly varies and greatly depends on how the supplier of a patch works with us. Many bugs in our bug tracker are like "this doesn't work for me, please do my work and fix it for me". Nearly noone even sends so much as a working reproduction. We're not a project like that. We need people needing things fixed to implement the fix. However, if you're talking about accepting and pushing code that easy for us to review, clear, and in good form, accepting should normally be a matter of some days. Regards, Simon
On 2023-09-21 18:39 +03:00, Tom Rini wrote: > On Wed, Sep 20, 2023 at 07:03:07PM -0600, Simon Glass wrote: >> Hi Maxim, >> >> On Thu, 14 Sept 2023 at 10:20, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >>> >>> add external lwIP library as a git submodule. If we add submodules, they should point to mirrors on source.denx.de, in case upstream repositories vanish. >> >> Oh dear...what is the motivation for using a submodule? > > That we don't have a better alternative. Using "git subtree" has it's > own problems and won't make things overall better. I need to raise a technical concern here, buildman uses "git worktree" to save disk space (provides git functionality in .bm-work source copies without copying .git into N such dirs), and there's this warning in the git-worktree(1) manual page: Multiple checkout in general is still experimental, and the support for submodules is incomplete. It is NOT recommended to make multiple checkouts of a superproject. It's been there for a while, and I don't know what the current status is. But if we decide to add submodules we'll most likely need to rework some stuff in buildman to support that. Besides that, I've been running into git submodule problems in other projects and don't like it at all. It doesn't seem to play nice with even fundamental git features/commands like git pull and git checkout. >> Our current stack is nicely integrated into U-Boot. This would make >> moving between development branches much more painful. > > It really shouldn't matter in that case. Unless you're trying out a new > lwip upstream commit, nothing changes in there. It _may_ mean that if > we want to update lwip it's not something that can be staged first in > the -next branch but instead something to pull in just after release, > but I'd need to see how smart or not git is today about things like > that. > >> I would much prefer that we bring in the necessary code, and that you >> send a patch every 3 months or so to deal with updates, making sure >> there are no code-size regressions. > > And I much prefer something that will make sure that we don't start > making changes in upstream code and diverging. I don't think there's a > mechanic short of submodule that will do that for us. Coreboot clones some payload repositories in Makefiles (although they use submodules a lot as well). Another alternative is having buildman manage external sources like how binman can download sources and build the tools it needs. Or maybe just assume the source already exists at ../lwip/ or $LWIP_SOURCE and raise an error if we can't find it there. I admit these might sound worse than git submodules, but at least any problems with them will be our fault and would be solvable in U-Boot instead of requiring a trip to upstream git.
diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000000..245ecff585 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "lwip"] + path = net/lwip/lwip-external + url = https://git.savannah.nongnu.org/git/lwip.git diff --git a/net/lwip/lwip-external b/net/lwip/lwip-external new file mode 160000 index 0000000000..84fde1ebbf --- /dev/null +++ b/net/lwip/lwip-external @@ -0,0 +1 @@ +Subproject commit 84fde1ebbfe35b3125fc2d89b8a456cbacf148e9
add external lwIP library as a git submodule. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- .gitmodules | 3 +++ net/lwip/lwip-external | 1 + 2 files changed, 4 insertions(+) create mode 100644 .gitmodules create mode 160000 net/lwip/lwip-external