[v4] doc/patches: add meson build to contributing guide

Message ID 20190124230541.58343-1-vipin.varghese@intel.com
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series
  • [v4] doc/patches: add meson build to contributing guide
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Varghese, Vipin Jan. 24, 2019, 11:05 p.m.
Patches has to be validated for meson devtool script for
code and document changes. Updating documentation for meson
build steps in checking Compilation category.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
Tested-by: Marko Kovacevic <marko.kovacevic@intel.com>
Acked-by: Marko Kovacevic <marko.kovacevic@intel.com>
---

V4:
 - spelling correction for Compilation - Thomas Monjalon
 - restored double space for new header - Thomas Monjalon
 - reword the meson compilation content - Vipin Varghese
 - Added 'tested' and 'acked' from - Marko Kovacevic

V3:
 removed extra character - Vipin Varghese

V2:
 updated the meson build options - Bruce Richardson
---
 doc/guides/contributing/patches.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Thomas Monjalon Jan. 28, 2019, 12:39 a.m. | #1
25/01/2019 00:05, Vipin Varghese:
> Patches has to be validated for meson devtool script for
> code and document changes. Updating documentation for meson
> build steps in checking Compilation category.
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> Tested-by: Marko Kovacevic <marko.kovacevic@intel.com>
> Acked-by: Marko Kovacevic <marko.kovacevic@intel.com>
> ---
> 
> V4:
>  - spelling correction for Compilation - Thomas Monjalon
>  - restored double space for new header - Thomas Monjalon
>  - reword the meson compilation content - Vipin Varghese
>  - Added 'tested' and 'acked' from - Marko Kovacevic
> 
> V3:
>  removed extra character - Vipin Varghese
> 
> V2:
>  updated the meson build options - Bruce Richardson
> ---
>  doc/guides/contributing/patches.rst | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
> index a64bb0368..e9048bbc0 100644
> --- a/doc/guides/contributing/patches.rst
> +++ b/doc/guides/contributing/patches.rst
> @@ -473,6 +473,14 @@ The recommended configurations and options to test compilation prior to submitti
>     export DPDK_DEP_PCAP=y
>     export DPDK_DEP_SSL=y

The lines above are about the "make system".
We need a transition to explain that the "meson system" is different.

> +Compilation of patches is to be tested with ``test-meson-builds.sh`` script
> +in ``devtools`` directory of the DPDK repo::

Would be more straight-forward to give the full path:
	devtools/test-meson-builds.sh
No need to add "DPDK repo".

> +
> +   devtools/test-meson-builds.sh

Do we really need to repeat the script name here?

> +
> +The script internally checks for dependencies and tool chain. Then builds with

tool chain -> toolchain

> +shared and static libraries for Linux and BSD targets.

Why "Linux and BSD" ? It is just testing for the running OS.
Varghese, Vipin Jan. 28, 2019, 2:27 p.m. | #2
Hi Thomas,

snipped 
> > +Compilation of patches is to be tested with ``test-meson-builds.sh``
> > +script in ``devtools`` directory of the DPDK repo::
> 
> Would be more straight-forward to give the full path:
> 	devtools/test-meson-builds.sh
I also like to use ' devtools/test-meson-builds.sh', but based I followed the formatting used for 'test-build.sh'. 

Which is ``` Compilation of patches and changes should be tested using the test-build.sh script in the devtools directory of the DPDK repo```. 

> No need to add "DPDK repo".
> 
> > +
> > +   devtools/test-meson-builds.sh
> 
> Do we really need to repeat the script name here?
Same as above. 

Note: please let me know if you still require the change for above items?

> 
> > +
> > +The script internally checks for dependencies and tool chain. Then
> > +builds with
> 
> tool chain -> toolchain
I will fix this and share the updated version.

> 
> > +shared and static libraries for Linux and BSD targets.
> 
> Why "Linux and BSD" ? It is just testing for the running OS.
The default execution generates for default targets as ' build-gcc-shared, build-gcc-static & build-x86-default'. If cross build is supported ' build-clang-shared & build-clang-static'.

As per my understanding this will true for Linux and BSD (Windows I am not aware). Hence I mention it as 'Linux and BSD'.

Note: 
1. If it is with regard to wording I can reword from ' Then builds with shared and static libraries for Linux and BSD targets' to 'Then binaries are built with static and shared library for default and arm cross build on Linux and BSD'
2.if support & built infrastructure works for Windows I can reword from ' Then builds with shared and static libraries for Linux and BSD targets' to 'Then binaries are built with static and shared library for default and arm cross build for Host OS'

Please let me know is 1 or 2 is right one?

> 
>
Thomas Monjalon Jan. 28, 2019, 2:52 p.m. | #3
28/01/2019 15:27, Varghese, Vipin:
> Hi Thomas,
> 
> snipped 
> > > +Compilation of patches is to be tested with ``test-meson-builds.sh``
> > > +script in ``devtools`` directory of the DPDK repo::
> > 
> > Would be more straight-forward to give the full path:
> > 	devtools/test-meson-builds.sh
> I also like to use ' devtools/test-meson-builds.sh', but based I followed the formatting used for 'test-build.sh'. 
> 
> Which is ``` Compilation of patches and changes should be tested using the test-build.sh script in the devtools directory of the DPDK repo```. 
> 
> > No need to add "DPDK repo".
> > 
> > > +
> > > +   devtools/test-meson-builds.sh
> > 
> > Do we really need to repeat the script name here?
> Same as above. 
> 
> Note: please let me know if you still require the change for above items?

Yes I've seen it was like that for make test.
Given the meson test script has no argument, it's better to make it
simple and just give the full path.

> > > +The script internally checks for dependencies and tool chain. Then
> > > +builds with
> > 
> > tool chain -> toolchain
> I will fix this and share the updated version.
> 
> > > +shared and static libraries for Linux and BSD targets.
> > 
> > Why "Linux and BSD" ? It is just testing for the running OS.
> The default execution generates for default targets as ' build-gcc-shared, build-gcc-static & build-x86-default'. If cross build is supported ' build-clang-shared & build-clang-static'.
> 
> As per my understanding this will true for Linux and BSD (Windows I am not aware). Hence I mention it as 'Linux and BSD'.
> 
> Note: 
> 1. If it is with regard to wording I can reword from ' Then builds with shared and static libraries for Linux and BSD targets' to 'Then binaries are built with static and shared library for default and arm cross build on Linux and BSD'
> 2.if support & built infrastructure works for Windows I can reword from ' Then builds with shared and static libraries for Linux and BSD targets' to 'Then binaries are built with static and shared library for default and arm cross build for Host OS'
> 
> Please let me know is 1 or 2 is right one?

We must avoid giving too much details because it will be outdated quickly
when updating the script. Please keep it simple and don't mention
Linux/BSD or Arm.
You can just explain the intent: it is testing several combinations of
compilation configurations.
Varghese, Vipin Jan. 28, 2019, 3:40 p.m. | #4
Thanks Thomas for the updates, I will check and send next version soon

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, January 28, 2019 8:23 PM
> To: Varghese, Vipin <vipin.varghese@intel.com>
> Cc: dev@dpdk.org; Mcnamara, John <john.mcnamara@intel.com>; Kovacevic,
> Marko <marko.kovacevic@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Padubidri, Sanjay A <sanjay.padubidri@intel.com>; Patel, Amol
> <amol.patel@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4] doc/patches: add meson build to
> contributing guide
> 
> 28/01/2019 15:27, Varghese, Vipin:
> > Hi Thomas,
> >
> > snipped
> > > > +Compilation of patches is to be tested with
> > > > +``test-meson-builds.sh`` script in ``devtools`` directory of the DPDK
> repo::
> > >
> > > Would be more straight-forward to give the full path:
> > > 	devtools/test-meson-builds.sh
> > I also like to use ' devtools/test-meson-builds.sh', but based I followed the
> formatting used for 'test-build.sh'.
> >
> > Which is ``` Compilation of patches and changes should be tested using the
> test-build.sh script in the devtools directory of the DPDK repo```.
> >
> > > No need to add "DPDK repo".
> > >
> > > > +
> > > > +   devtools/test-meson-builds.sh
> > >
> > > Do we really need to repeat the script name here?
> > Same as above.
> >
> > Note: please let me know if you still require the change for above items?
> 
> Yes I've seen it was like that for make test.
> Given the meson test script has no argument, it's better to make it simple and
> just give the full path.
> 
> > > > +The script internally checks for dependencies and tool chain.
> > > > +Then builds with
> > >
> > > tool chain -> toolchain
> > I will fix this and share the updated version.
> >
> > > > +shared and static libraries for Linux and BSD targets.
> > >
> > > Why "Linux and BSD" ? It is just testing for the running OS.
> > The default execution generates for default targets as ' build-gcc-shared,
> build-gcc-static & build-x86-default'. If cross build is supported ' build-clang-
> shared & build-clang-static'.
> >
> > As per my understanding this will true for Linux and BSD (Windows I am not
> aware). Hence I mention it as 'Linux and BSD'.
> >
> > Note:
> > 1. If it is with regard to wording I can reword from ' Then builds with shared
> and static libraries for Linux and BSD targets' to 'Then binaries are built with
> static and shared library for default and arm cross build on Linux and BSD'
> > 2.if support & built infrastructure works for Windows I can reword from '
> Then builds with shared and static libraries for Linux and BSD targets' to 'Then
> binaries are built with static and shared library for default and arm cross build
> for Host OS'
> >
> > Please let me know is 1 or 2 is right one?
> 
> We must avoid giving too much details because it will be outdated quickly
> when updating the script. Please keep it simple and don't mention Linux/BSD
> or Arm.
> You can just explain the intent: it is testing several combinations of
> compilation configurations.
> 
>

Patch

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index a64bb0368..e9048bbc0 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -473,6 +473,14 @@  The recommended configurations and options to test compilation prior to submitti
    export DPDK_DEP_PCAP=y
    export DPDK_DEP_SSL=y
 
+Compilation of patches is to be tested with ``test-meson-builds.sh`` script
+in ``devtools`` directory of the DPDK repo::
+
+   devtools/test-meson-builds.sh
+
+The script internally checks for dependencies and tool chain. Then builds with
+shared and static libraries for Linux and BSD targets.
+
 
 Sending Patches
 ---------------