[dpdk-dev] mk: remove make target for examples

Message ID 1479772058-7112-1-git-send-email-thomas.monjalon@6wind.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
checkpatch/checkpatch success coding style OK

Commit Message

Thomas Monjalon Nov. 21, 2016, 11:47 p.m. UTC
  The command
  make examples
works only if target directories have the exact name of configs.

It is more flexible to use
  make -C examples RTE_SDK=$(pwd) RTE_TARGET=build

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 mk/rte.sdkexamples.mk | 77 ---------------------------------------------------
 mk/rte.sdkroot.mk     |  4 ---
 2 files changed, 81 deletions(-)
 delete mode 100644 mk/rte.sdkexamples.mk
  

Comments

Ferruh Yigit Nov. 22, 2016, 12:34 a.m. UTC | #1
On 11/21/2016 11:47 PM, Thomas Monjalon wrote:
> The command
>   make examples
> works only if target directories have the exact name of configs.
> 
> It is more flexible to use
>   make -C examples RTE_SDK=$(pwd) RTE_TARGET=build
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Instead of removing examples & examples_clean targets, what do you think
keeping them as wrapper to suggested usage, for backward compatibility.

Something like:
"
BUILDING_RTE_SDK :=
export BUILDING_RTE_SDK

# Build directory is given with O=
O ?= $(RTE_SDK)/examples

# Target for which examples should be built.
T ?= build

.PHONY: examples
examples:
        @echo ================== Build examples for $(T)
        $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T);

.PHONY: examples_clean
examples_clean:
        @echo ================== Clean examples for $(T)
        $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T) clean;
"
  
Thomas Monjalon Nov. 22, 2016, 9:38 a.m. UTC | #2
2016-11-22 00:34, Ferruh Yigit:
> On 11/21/2016 11:47 PM, Thomas Monjalon wrote:
> > The command
> >   make examples
> > works only if target directories have the exact name of configs.
> > 
> > It is more flexible to use
> >   make -C examples RTE_SDK=$(pwd) RTE_TARGET=build
> > 
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> Instead of removing examples & examples_clean targets, what do you think
> keeping them as wrapper to suggested usage, for backward compatibility.
> 
> Something like:
> "
> BUILDING_RTE_SDK :=
> export BUILDING_RTE_SDK
> 
> # Build directory is given with O=
> O ?= $(RTE_SDK)/examples
> 
> # Target for which examples should be built.
> T ?= build
> 
> .PHONY: examples
> examples:
>         @echo ================== Build examples for $(T)
>         $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T);
> 
> .PHONY: examples_clean
> examples_clean:
>         @echo ================== Clean examples for $(T)
>         $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T) clean;
> "

What is the benefit of this makefile? Just remove -C ?
It is not compatible with the old behaviour, so I'm afraid it would be
confusing for no real benefit.
  
Ferruh Yigit Nov. 22, 2016, 11:49 a.m. UTC | #3
On 11/22/2016 9:38 AM, Thomas Monjalon wrote:
> 2016-11-22 00:34, Ferruh Yigit:
>> On 11/21/2016 11:47 PM, Thomas Monjalon wrote:
>>> The command
>>>   make examples
>>> works only if target directories have the exact name of configs.
>>>
>>> It is more flexible to use
>>>   make -C examples RTE_SDK=$(pwd) RTE_TARGET=build
>>>
>>> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
>>
>> Instead of removing examples & examples_clean targets, what do you think
>> keeping them as wrapper to suggested usage, for backward compatibility.
>>
>> Something like:
>> "
>> BUILDING_RTE_SDK :=
>> export BUILDING_RTE_SDK
>>
>> # Build directory is given with O=
>> O ?= $(RTE_SDK)/examples
>>
>> # Target for which examples should be built.
>> T ?= build
>>
>> .PHONY: examples
>> examples:
>>         @echo ================== Build examples for $(T)
>>         $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T);
>>
>> .PHONY: examples_clean
>> examples_clean:
>>         @echo ================== Clean examples for $(T)
>>         $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T) clean;
>> "
> 
> What is the benefit of this makefile? Just remove -C ?

To keep existing targets, in case somebody use them.

> It is not compatible with the old behaviour, so I'm afraid it would be
> confusing for no real benefit.

Right, not fully compatible, but still can do:
make examples / make examples_clean
make examples T=x86_64-native-linuxapp-gcc

Overall, if you believe keeping them is confusing, I am OK with it, just
may need to update doc/build-sdk-quick.txt to fix "make help" output.

Thanks,
ferruh
  
Ferruh Yigit Jan. 11, 2019, 10:01 p.m. UTC | #4
On 11/22/2016 11:49 AM, ferruh.yigit at intel.com (Ferruh Yigit) wrote:
> On 11/22/2016 9:38 AM, Thomas Monjalon wrote:
>> 2016-11-22 00:34, Ferruh Yigit:
>>> On 11/21/2016 11:47 PM, Thomas Monjalon wrote:
>>>> The command
>>>>   make examples
>>>> works only if target directories have the exact name of configs.
>>>>
>>>> It is more flexible to use
>>>>   make -C examples RTE_SDK=$(pwd) RTE_TARGET=build
>>>>
>>>> Signed-off-by: Thomas Monjalon <thomas.monjalon at 6wind.com>
>>>
>>> Instead of removing examples & examples_clean targets, what do you think
>>> keeping them as wrapper to suggested usage, for backward compatibility.
>>>
>>> Something like:
>>> "
>>> BUILDING_RTE_SDK :=
>>> export BUILDING_RTE_SDK
>>>
>>> # Build directory is given with O=
>>> O ?= $(RTE_SDK)/examples
>>>
>>> # Target for which examples should be built.
>>> T ?= build
>>>
>>> .PHONY: examples
>>> examples:
>>>         @echo ================== Build examples for $(T)
>>>         $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T);
>>>
>>> .PHONY: examples_clean
>>> examples_clean:
>>>         @echo ================== Clean examples for $(T)
>>>         $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T) clean;
>>> "
>>
>> What is the benefit of this makefile? Just remove -C ?
> 
> To keep existing targets, in case somebody use them.
> 
>> It is not compatible with the old behaviour, so I'm afraid it would be
>> confusing for no real benefit.
> 
> Right, not fully compatible, but still can do:
> make examples / make examples_clean
> make examples T=x86_64-native-linuxapp-gcc
> 
> Overall, if you believe keeping them is confusing, I am OK with it, just
> may need to update doc/build-sdk-quick.txt to fix "make help" output.

Hi Thomas,

There is no update on the patch for a long time, updating it as rejected, please
send an updated version if it is still relevant.

For record, patch: https://patches.dpdk.org/patch/17174/
  
Thomas Monjalon Jan. 11, 2019, 10:13 p.m. UTC | #5
11/01/2019 23:01, Ferruh Yigit:
> On 11/22/2016 11:49 AM, ferruh.yigit at intel.com (Ferruh Yigit) wrote:
> > On 11/22/2016 9:38 AM, Thomas Monjalon wrote:
> >> 2016-11-22 00:34, Ferruh Yigit:
> >>> On 11/21/2016 11:47 PM, Thomas Monjalon wrote:
> >>>> The command
> >>>>   make examples
> >>>> works only if target directories have the exact name of configs.
> >>>>
> >>>> It is more flexible to use
> >>>>   make -C examples RTE_SDK=$(pwd) RTE_TARGET=build
> >>>>
> >>>> Signed-off-by: Thomas Monjalon <thomas.monjalon at 6wind.com>
> >>>
> >>> Instead of removing examples & examples_clean targets, what do you think
> >>> keeping them as wrapper to suggested usage, for backward compatibility.
> >>>
> >>> Something like:
> >>> "
> >>> BUILDING_RTE_SDK :=
> >>> export BUILDING_RTE_SDK
> >>>
> >>> # Build directory is given with O=
> >>> O ?= $(RTE_SDK)/examples
> >>>
> >>> # Target for which examples should be built.
> >>> T ?= build
> >>>
> >>> .PHONY: examples
> >>> examples:
> >>>         @echo ================== Build examples for $(T)
> >>>         $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T);
> >>>
> >>> .PHONY: examples_clean
> >>> examples_clean:
> >>>         @echo ================== Clean examples for $(T)
> >>>         $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T) clean;
> >>> "
> >>
> >> What is the benefit of this makefile? Just remove -C ?
> > 
> > To keep existing targets, in case somebody use them.
> > 
> >> It is not compatible with the old behaviour, so I'm afraid it would be
> >> confusing for no real benefit.
> > 
> > Right, not fully compatible, but still can do:
> > make examples / make examples_clean
> > make examples T=x86_64-native-linuxapp-gcc
> > 
> > Overall, if you believe keeping them is confusing, I am OK with it, just
> > may need to update doc/build-sdk-quick.txt to fix "make help" output.
> 
> Hi Thomas,
> 
> There is no update on the patch for a long time, updating it as rejected, please
> send an updated version if it is still relevant.
> 
> For record, patch: https://patches.dpdk.org/patch/17174/

The goal was a clean-up.
It won't be relevant anymore when the make-based build will be removed.
So it's not relevant anymore. Let's focus on meson build.
  

Patch

diff --git a/mk/rte.sdkexamples.mk b/mk/rte.sdkexamples.mk
deleted file mode 100644
index 111ce91..0000000
--- a/mk/rte.sdkexamples.mk
+++ /dev/null
@@ -1,77 +0,0 @@ 
-#   BSD LICENSE
-#
-#   Copyright(c) 2014 6WIND S.A.
-#
-#   Redistribution and use in source and binary forms, with or without
-#   modification, are permitted provided that the following conditions
-#   are met:
-#
-#     * Redistributions of source code must retain the above copyright
-#       notice, this list of conditions and the following disclaimer.
-#     * Redistributions in binary form must reproduce the above copyright
-#       notice, this list of conditions and the following disclaimer in
-#       the documentation and/or other materials provided with the
-#       distribution.
-#     * Neither the name of 6WIND S.A. nor the names of its
-#       contributors may be used to endorse or promote products derived
-#       from this software without specific prior written permission.
-#
-#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-# examples application are seen as external applications which are
-# not part of SDK.
-BUILDING_RTE_SDK :=
-export BUILDING_RTE_SDK
-
-# Build directory is given with O=
-O ?= $(RTE_SDK)/examples
-
-# Target for which examples should be built.
-T ?= *
-
-# list all available configurations
-EXAMPLES_CONFIGS := $(patsubst $(RTE_SRCDIR)/config/defconfig_%,%,\
-	$(wildcard $(RTE_SRCDIR)/config/defconfig_$(T)))
-EXAMPLES_TARGETS := $(addsuffix _examples,\
-	$(filter-out %~,$(EXAMPLES_CONFIGS)))
-
-.PHONY: examples
-examples: $(EXAMPLES_TARGETS)
-
-%_examples:
-	@echo ================== Build examples for $*
-	$(Q)if [ ! -d "${RTE_SDK}/${*}" ]; then \
-		echo "Target ${*} does not exist in ${RTE_SDK}/${*}." ; \
-		echo -n "Please install DPDK first (make install) or use another " ; \
-		echo "target argument (T=target)." ; \
-		false ; \
-	else \
-		$(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(*); \
-	fi
-
-EXAMPLES_CLEAN_TARGETS := $(addsuffix _examples_clean,\
-	$(filter-out %~,$(EXAMPLES_CONFIGS)))
-
-.PHONY: examples_clean
-examples_clean: $(EXAMPLES_CLEAN_TARGETS)
-
-%_examples_clean:
-	@echo ================== Clean examples for $*
-	$(Q)if [ ! -d "${RTE_SDK}/${*}" ]; then \
-		echo "Target ${*} does not exist in ${RTE_SDK}/${*}." ; \
-		echo -n "Please install DPDK first (make install) or use another " ; \
-		echo "target argument (T=target)." ; \
-		false ; \
-	else \
-		$(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(*) clean; \
-	fi
diff --git a/mk/rte.sdkroot.mk b/mk/rte.sdkroot.mk
index 04ad523..81233ed 100644
--- a/mk/rte.sdkroot.mk
+++ b/mk/rte.sdkroot.mk
@@ -117,10 +117,6 @@  depdirs depgraph:
 gcov gcovclean:
 	$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdkgcov.mk $@
 
-.PHONY: examples examples_clean
-examples examples_clean:
-	$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdkexamples.mk $@
-
 # all other build targets
 %:
 	$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdkconfig.mk checkconfig