diff mbox series

[RFC,1/5] add lwip-external submodule

Message ID 20230505102529.1254445-2-maxim.uvarov@linaro.org
State Superseded
Headers show
Series LWIP stack integration | expand

Commit Message

Maxim Uvarov May 5, 2023, 10:25 a.m. UTC
Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 .gitmodules            | 3 +++
 lib/lwip/lwip-external | 1 +
 2 files changed, 4 insertions(+)
 create mode 100644 .gitmodules
 create mode 160000 lib/lwip/lwip-external

Comments

Simon Glass May 8, 2023, 2:43 p.m. UTC | #1
Hi Maxim,

On Fri, 5 May 2023 at 04:50, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  .gitmodules            | 3 +++
>  lib/lwip/lwip-external | 1 +
>  2 files changed, 4 insertions(+)
>  create mode 100644 .gitmodules
>  create mode 160000 lib/lwip/lwip-external
>

Please no submodules. They are such a pain. If we want the code in
U-Boot, let's put it in U-Boot and upstream our changes as needed.

Regards,
Simon
Ilias Apalodimas May 10, 2023, 7:40 a.m. UTC | #2
Hi Simon,

On Mon, May 08, 2023 at 08:43:14AM -0600, Simon Glass wrote:
> Hi Maxim,
>
> On Fri, 5 May 2023 at 04:50, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> >
> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> > ---
> >  .gitmodules            | 3 +++
> >  lib/lwip/lwip-external | 1 +
> >  2 files changed, 4 insertions(+)
> >  create mode 100644 .gitmodules
> >  create mode 160000 lib/lwip/lwip-external
> >
>
> Please no submodules. They are such a pain. If we want the code in
> U-Boot, let's put it in U-Boot and upstream our changes as needed.

Can you explain a bit more the pain points you are seeing in u-boot with
submodules?  EDK2 does submodules for openSSL and it's quite convenient,
since you dont have to maintain any code, do backports etc.  Instead we can
just use upstream projects as-is.
IMHO we should work on having it as an experimental feature in parallel
with the current TCP efforts for a while and have a Kconfig switch.  If we
are happy in the long run and the code increase isn't prohibitive, we can
consider switching permanently

Regards
/Ilias
>
> Regards,
> Simon
Peter Robinson May 10, 2023, 2:46 p.m. UTC | #3
On Wed, May 10, 2023 at 8:40 AM Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Simon,
>
> On Mon, May 08, 2023 at 08:43:14AM -0600, Simon Glass wrote:
> > Hi Maxim,
> >
> > On Fri, 5 May 2023 at 04:50, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > >
> > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > ---
> > >  .gitmodules            | 3 +++
> > >  lib/lwip/lwip-external | 1 +
> > >  2 files changed, 4 insertions(+)
> > >  create mode 100644 .gitmodules
> > >  create mode 160000 lib/lwip/lwip-external
> > >
> >
> > Please no submodules. They are such a pain. If we want the code in
> > U-Boot, let's put it in U-Boot and upstream our changes as needed.
>
> Can you explain a bit more the pain points you are seeing in u-boot with
> submodules?  EDK2 does submodules for openSSL and it's quite convenient,
> since you dont have to maintain any code, do backports etc.  Instead we can
> just use upstream projects as-is.

I feel there's pros and cons for both, similarly different projects
have different projects have different policies. Tom may have a more
definite opinion.

> IMHO we should work on having it as an experimental feature in parallel
> with the current TCP efforts for a while and have a Kconfig switch.  If we
> are happy in the long run and the code increase isn't prohibitive, we can
> consider switching permanently

At least for initial review of the prototype I don't see it as
blocking for people to get a general idea what is going on.

Peter
Tom Rini May 11, 2023, 1:51 p.m. UTC | #4
On Wed, May 10, 2023 at 03:46:31PM +0100, Peter Robinson wrote:
> On Wed, May 10, 2023 at 8:40 AM Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Mon, May 08, 2023 at 08:43:14AM -0600, Simon Glass wrote:
> > > Hi Maxim,
> > >
> > > On Fri, 5 May 2023 at 04:50, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > >
> > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > > ---
> > > >  .gitmodules            | 3 +++
> > > >  lib/lwip/lwip-external | 1 +
> > > >  2 files changed, 4 insertions(+)
> > > >  create mode 100644 .gitmodules
> > > >  create mode 160000 lib/lwip/lwip-external
> > > >
> > >
> > > Please no submodules. They are such a pain. If we want the code in
> > > U-Boot, let's put it in U-Boot and upstream our changes as needed.
> >
> > Can you explain a bit more the pain points you are seeing in u-boot with
> > submodules?  EDK2 does submodules for openSSL and it's quite convenient,
> > since you dont have to maintain any code, do backports etc.  Instead we can
> > just use upstream projects as-is.
> 
> I feel there's pros and cons for both, similarly different projects
> have different projects have different policies. Tom may have a more
> definite opinion.
> 
> > IMHO we should work on having it as an experimental feature in parallel
> > with the current TCP efforts for a while and have a Kconfig switch.  If we
> > are happy in the long run and the code increase isn't prohibitive, we can
> > consider switching permanently
> 
> At least for initial review of the prototype I don't see it as
> blocking for people to get a general idea what is going on.

Yeah, I also don't like submodules. But for the purpose of this RFC, at
this stage in the RFC, it's fine.  May or may not be easier to review.
Tom Rini May 11, 2023, 1:51 p.m. UTC | #5
On Fri, May 05, 2023 at 10:25:25AM +0000, Maxim Uvarov wrote:

> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  .gitmodules            | 3 +++
>  lib/lwip/lwip-external | 1 +
>  2 files changed, 4 insertions(+)
>  create mode 100644 .gitmodules
>  create mode 160000 lib/lwip/lwip-external
> 
> diff --git a/.gitmodules b/.gitmodules
> new file mode 100644
> index 0000000000..afc709af10
> --- /dev/null
> +++ b/.gitmodules
> @@ -0,0 +1,3 @@
> +[submodule "lib/lwip/lwip-external"]
> +	path = lib/lwip/lwip-external
> +	url = https://git.savannah.nongnu.org/git/lwip.git
> diff --git a/lib/lwip/lwip-external b/lib/lwip/lwip-external
> new file mode 160000
> index 0000000000..3fe8d2fc43
> --- /dev/null
> +++ b/lib/lwip/lwip-external
> @@ -0,0 +1 @@
> +Subproject commit 3fe8d2fc43a9b69f7ed28c63d44a7744f9c0def9

Why are we using top of tree here, rather than say STABLE-2_1_3_RELEASE
tag? Is there some feature we need post that release? I'd be much more
inclined to track releases than top of trees for a project which does
have (from a quick look) release tags.
diff mbox series

Patch

diff --git a/.gitmodules b/.gitmodules
new file mode 100644
index 0000000000..afc709af10
--- /dev/null
+++ b/.gitmodules
@@ -0,0 +1,3 @@ 
+[submodule "lib/lwip/lwip-external"]
+	path = lib/lwip/lwip-external
+	url = https://git.savannah.nongnu.org/git/lwip.git
diff --git a/lib/lwip/lwip-external b/lib/lwip/lwip-external
new file mode 160000
index 0000000000..3fe8d2fc43
--- /dev/null
+++ b/lib/lwip/lwip-external
@@ -0,0 +1 @@ 
+Subproject commit 3fe8d2fc43a9b69f7ed28c63d44a7744f9c0def9