[PATCH 0/2] Add edk2 submodule policy

[PATCH 0/2] Add edk2 submodule policy

[Wang, Jian J]

https://bugzilla.tianocore.org/show_bug.cgi?id=1910

Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>

Jian J Wang (2):
  Readme.md: add submodule policy and clone commands
  CryptoPkg/OpensslLib: remove clone commands

 .../Library/OpensslLib/OpenSSL-HOWTO.txt      | 18 ++----------------
 Readme.md                                     | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 16 deletions(-)

-- 
2.17.1.windows.2

[PATCH 1/2] Readme.md: add submodule policy and clone commands

[Wang, Jian J]

https://bugzilla.tianocore.org/show_bug.cgi?id=1910

A section 'Submodules' is added to clarify the submodule policy
in edk2 repo. Git commands are also added to show the correct
way to clone submodule repos, in which '--recursive' is removed
because it's not needed but recommended in other document.

Related commits:
Openssl-1.1.1b upgrade: acfb90911840c38a0beb9bcfe0065668244d2b4d
berkeley-softfloat-3:   3cc57695df5a6e8c65fb46b993836c315cabf49d

Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 Readme.md | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Readme.md b/Readme.md
index e564c6c09b..ddb4da5648 100644
--- a/Readme.md
+++ b/Readme.md
@@ -143,3 +143,22 @@ Signed-off-by: Contributor Name <contributor@example.com>
   the change.  Each line should be less than ~70 characters.
 * `Signed-off-by` is the contributor's signature identifying them
   by their real/legal name and their email address.
+
+# Submodules
+
+As a general policy, submodules should be avoided in EDK II repo as possible as we can, especially submodules required by other submodules. Currently EDK II  contains two submodules
+
+- CryptoPkg/Library/OpensslLib/openssl
+- ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3
+
+The later one is actually required by previous one. It's inevitable in openssl-1.1.1 (since stable201905) for floating point parameter conversion, but should be dropped once there's no such need in future release of openssl.
+
+Note: When cloning submodule repos, '--recursive' option is not recommended. EDK II itself will not use any code/feature from submodules in above submodules. '--recursive' might cause failure in cloning behind proxy.
+
+To get a full, buildable EDK II repository, use following two steps of git command
+
+```
+$ git clone https://github.com/tianocore/edk2.git
+$ git submodule update --init
+```
+
-- 
2.17.1.windows.2

Re: [PATCH 1/2] Readme.md: add submodule policy and clone commands

[Leif Lindholm]

Hi Jian,

Many thanks for this.
A few comments on the text.

On Tue, Jul 09, 2019 at 02:36:00PM +0800, Jian J Wang wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1910
> 
> A section 'Submodules' is added to clarify the submodule policy
> in edk2 repo. Git commands are also added to show the correct
> way to clone submodule repos, in which '--recursive' is removed
> because it's not needed but recommended in other document.
> 
> Related commits:
> Openssl-1.1.1b upgrade: acfb90911840c38a0beb9bcfe0065668244d2b4d
> berkeley-softfloat-3:   3cc57695df5a6e8c65fb46b993836c315cabf49d
> 
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>  Readme.md | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Readme.md b/Readme.md
> index e564c6c09b..ddb4da5648 100644
> --- a/Readme.md
> +++ b/Readme.md
> @@ -143,3 +143,22 @@ Signed-off-by: Contributor Name <contributor@example.com>
>    the change.  Each line should be less than ~70 characters.
>  * `Signed-off-by` is the contributor's signature identifying them
>    by their real/legal name and their email address.
> +
> +# Submodules
> +
> +As a general policy, submodules should be avoided in EDK II repo as possible as we can, especially submodules required by other submodules. Currently EDK II  contains two submodules

Please wrap lines at 80 characters.
One of the benefits of markdown is that it can be easily read both as
a plain text file, and be rendered into something else (like html for
displaying on the web).

> +
> +- CryptoPkg/Library/OpensslLib/openssl
> +- ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3
> +
> +The later one is actually required by previous one. It's inevitable in openssl-1.1.1 (since stable201905) for floating point parameter conversion, but should be dropped once there's no such need in future release of openssl.

"later" -> "latter" when referring to items in an enumeration
(interesting quirk of English)

> +
> +Note: When cloning submodule repos, '--recursive' option is not recommended. EDK II itself will not use any code/feature from submodules in above submodules. '--recursive' might cause failure in cloning behind proxy.

I wouldn't say "behind proxy". It may fail because it adds additional
servers that must be reachable in order for the clone to succeed.

So we could replace the last sentence above with something like:
"So using '--recursive' adds a dependency on being able to reach
servers we do not actually want any code from, as well as needlessly
downloading code we will not use."

I would also prefer to move this "Note:" to immediately below the
command example:

> +
> +To get a full, buildable EDK II repository, use following two steps of git command
> +
> +```
> +$ git clone https://github.com/tianocore/edk2.git
> +$ git submodule update --init
> +```
> +

("Note:" here.)

/
    Leif

> -- 
> 2.17.1.windows.2
> 

Re: [PATCH 1/2] Readme.md: add submodule policy and clone commands

[Wang, Jian J]

Leif,

> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Tuesday, July 09, 2019 5:27 PM
> To: Wang, Jian J <jian.j.wang@intel.com>
> Cc: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [PATCH 1/2] Readme.md: add submodule policy and clone
> commands
> 
> Hi Jian,
> 
> Many thanks for this.
> A few comments on the text.
> 
> On Tue, Jul 09, 2019 at 02:36:00PM +0800, Jian J Wang wrote:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1910
> >
> > A section 'Submodules' is added to clarify the submodule policy
> > in edk2 repo. Git commands are also added to show the correct
> > way to clone submodule repos, in which '--recursive' is removed
> > because it's not needed but recommended in other document.
> >
> > Related commits:
> > Openssl-1.1.1b upgrade: acfb90911840c38a0beb9bcfe0065668244d2b4d
> > berkeley-softfloat-3:   3cc57695df5a6e8c65fb46b993836c315cabf49d
> >
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > ---
> >  Readme.md | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/Readme.md b/Readme.md
> > index e564c6c09b..ddb4da5648 100644
> > --- a/Readme.md
> > +++ b/Readme.md
> > @@ -143,3 +143,22 @@ Signed-off-by: Contributor Name
> <contributor@example.com>
> >    the change.  Each line should be less than ~70 characters.
> >  * `Signed-off-by` is the contributor's signature identifying them
> >    by their real/legal name and their email address.
> > +
> > +# Submodules
> > +
> > +As a general policy, submodules should be avoided in EDK II repo as
> possible as we can, especially submodules required by other submodules.
> Currently EDK II  contains two submodules
> 
> Please wrap lines at 80 characters.
> One of the benefits of markdown is that it can be easily read both as
> a plain text file, and be rendered into something else (like html for
> displaying on the web).
> 

I used markdown editor to do this. I didn't notice the line length. I'll wrap
it.

> > +
> > +- CryptoPkg/Library/OpensslLib/openssl
> > +- ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3
> > +
> > +The later one is actually required by previous one. It's inevitable in
> openssl-1.1.1 (since stable201905) for floating point parameter conversion,
> but should be dropped once there's no such need in future release of
> openssl.
> 
> "later" -> "latter" when referring to items in an enumeration
> (interesting quirk of English)
> 

Thanks for catching this. I might have used this wrong word for years:(

> > +
> > +Note: When cloning submodule repos, '--recursive' option is not
> recommended. EDK II itself will not use any code/feature from submodules
> in above submodules. '--recursive' might cause failure in cloning behind
> proxy.
> 
> I wouldn't say "behind proxy". It may fail because it adds additional
> servers that must be reachable in order for the clone to succeed.
> 
> So we could replace the last sentence above with something like:
> "So using '--recursive' adds a dependency on being able to reach
> servers we do not actually want any code from, as well as needlessly
> downloading code we will not use."
> 
> I would also prefer to move this "Note:" to immediately below the
> command example:

You're right. I'll use your version instead in v2. Thanks for the comments.

Regards,
Jian
> 
> > +
> > +To get a full, buildable EDK II repository, use following two steps of git
> command
> > +
> > +```
> > +$ git clone https://github.com/tianocore/edk2.git
> > +$ git submodule update --init
> > +```
> > +
> 
> ("Note:" here.)
> 
> /
>     Leif
> 
> > --
> > 2.17.1.windows.2
> >

[PATCH 2/2] CryptoPkg/OpensslLib: remove clone commands

[Wang, Jian J]

https://bugzilla.tianocore.org/show_bug.cgi?id=1910

edk2/Readme.md has added a section to explain the correct clone
commands for submodules. Detailed steps in the OpenSSL-HOWTO.txt
are removed to avoid any inconsistency.

Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 CryptoPkg/Library/OpensslLib/OpenSSL-HOWTO.txt | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/CryptoPkg/Library/OpensslLib/OpenSSL-HOWTO.txt b/CryptoPkg/Library/OpensslLib/OpenSSL-HOWTO.txt
index db45eb88d1..e52ee27b49 100644
--- a/CryptoPkg/Library/OpensslLib/OpenSSL-HOWTO.txt
+++ b/CryptoPkg/Library/OpensslLib/OpenSSL-HOWTO.txt
@@ -24,22 +24,8 @@ on the cryptography.
 =============================================================================
                       HOW to Install OpenSSL for UEFI Building
 =============================================================================
-  OpenSSL repository was added as one submodule of EDKII project.
-
-  The user can use the following commands to clone both main EDKII repo and
-openssl submodule:
-  1) Add the "--recursive" flag to the git clone command:
-     $ git clone --recursive https://github.com/tianocore/edk2
-or
-  2) Manually initialize and update the submodules after the clone operation
-     on main project:
-     $ git clone https://github.com/tianocore/edk2
-     $ git submodule update --init --recursive
-
-  And use the following combined commands to pull the remote submodule updates
-(e.g. Updating the new supported OpenSSL release tag):
-     $ git pull --recurse-submodules && \
-       git submodule update --recursive
+  OpenSSL repository was added as one submodule of EDKII project. Please
+refer to edk2/Readme.md for how to clone the code.
 
 =============================================================================
                       About process_files.pl
-- 
2.17.1.windows.2

Re: [PATCH 2/2] CryptoPkg/OpensslLib: remove clone commands

[Leif Lindholm]

On Tue, Jul 09, 2019 at 02:36:01PM +0800, Jian J Wang wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1910
> 
> edk2/Readme.md has added a section to explain the correct clone
> commands for submodules. Detailed steps in the OpenSSL-HOWTO.txt
> are removed to avoid any inconsistency.
> 
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  CryptoPkg/Library/OpensslLib/OpenSSL-HOWTO.txt | 18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/CryptoPkg/Library/OpensslLib/OpenSSL-HOWTO.txt b/CryptoPkg/Library/OpensslLib/OpenSSL-HOWTO.txt
> index db45eb88d1..e52ee27b49 100644
> --- a/CryptoPkg/Library/OpensslLib/OpenSSL-HOWTO.txt
> +++ b/CryptoPkg/Library/OpensslLib/OpenSSL-HOWTO.txt
> @@ -24,22 +24,8 @@ on the cryptography.
>  =============================================================================
>                        HOW to Install OpenSSL for UEFI Building
>  =============================================================================
> -  OpenSSL repository was added as one submodule of EDKII project.
> -
> -  The user can use the following commands to clone both main EDKII repo and
> -openssl submodule:
> -  1) Add the "--recursive" flag to the git clone command:
> -     $ git clone --recursive https://github.com/tianocore/edk2
> -or
> -  2) Manually initialize and update the submodules after the clone operation
> -     on main project:
> -     $ git clone https://github.com/tianocore/edk2
> -     $ git submodule update --init --recursive
> -
> -  And use the following combined commands to pull the remote submodule updates
> -(e.g. Updating the new supported OpenSSL release tag):
> -     $ git pull --recurse-submodules && \
> -       git submodule update --recursive
> +  OpenSSL repository was added as one submodule of EDKII project. Please
> +refer to edk2/Readme.md for how to clone the code.
>  
>  =============================================================================
>                        About process_files.pl
> -- 
> 2.17.1.windows.2
>