DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] mk: allow updates to build config on make install
@ 2014-05-14 10:22 Bruce Richardson
  2014-05-14 10:33 ` Thomas Monjalon
  2014-05-14 15:55 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
  0 siblings, 2 replies; 15+ messages in thread
From: Bruce Richardson @ 2014-05-14 10:22 UTC (permalink / raw)
  To: dev

There was an issue with rebuilding the code following a change to
one of the config files inside the "config" directory. If one did
a "make install T=<target>" and then made a modification to the
defconfig_<target> file (or applied a patch which modified that file)
a subsequent re-run of the make install command would not rebuild
the .config file leading to either build failures or an incorrect
build. This change fixes that issue.

Signed-off-by: Bruce Richardson<bruce.richardson@intel.com>
---
 mk/rte.sdkinstall.mk |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index 61a8771..00cb616 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -56,9 +56,7 @@ install: $(INSTALL_TARGETS)
 
 %_install:
 	@echo ================== Installing $*
-	$(Q)if [ ! -f $(BUILD_DIR)/$*/.config ]; then \
-		$(MAKE) config T=$* O=$(BUILD_DIR)/$*; \
-	fi
+	$(Q)$(MAKE) config T=$* O=$(BUILD_DIR)/$*
 	$(Q)$(MAKE) all O=$(BUILD_DIR)/$*
 
 #
-- 
1.7.0.7

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: allow updates to build config on make install
  2014-05-14 10:22 [dpdk-dev] [PATCH] mk: allow updates to build config on make install Bruce Richardson
@ 2014-05-14 10:33 ` Thomas Monjalon
  2014-05-14 10:51   ` Richardson, Bruce
  2014-05-14 15:55 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
  1 sibling, 1 reply; 15+ messages in thread
From: Thomas Monjalon @ 2014-05-14 10:33 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

Hi Bruce

2014-05-14 11:22, Bruce Richardson:
> There was an issue with rebuilding the code following a change to
> one of the config files inside the "config" directory. If one did
> a "make install T=<target>" and then made a modification to the
> defconfig_<target> file (or applied a patch which modified that file)
> a subsequent re-run of the make install command would not rebuild
> the .config file leading to either build failures or an incorrect
> build. This change fixes that issue.

Your patch is reverting this one:
	mk: in install rule, don't overwrite .config if it already exists
	http://dpdk.org/browse/dpdk/commit/?id=1c858a7dfebd4e4092eb55
As stated in the commit log,
	"This allows the user to prepare a configuration with make config
	before using make install."

So your patch is introducing a regression.

I think you are describing something which is not a bug.
If you make a modification to the configuration template, you must explicitly 
call "make config".

-- 
Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: allow updates to build config on make install
  2014-05-14 10:33 ` Thomas Monjalon
@ 2014-05-14 10:51   ` Richardson, Bruce
  2014-05-14 11:55     ` Thomas Monjalon
  0 siblings, 1 reply; 15+ messages in thread
From: Richardson, Bruce @ 2014-05-14 10:51 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, May 14, 2014 11:34 AM
> To: Richardson, Bruce
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] mk: allow updates to build config on make
> install
> 
> Hi Bruce
> 
> 2014-05-14 11:22, Bruce Richardson:
> > There was an issue with rebuilding the code following a change to one
> > of the config files inside the "config" directory. If one did a "make
> > install T=<target>" and then made a modification to the
> > defconfig_<target> file (or applied a patch which modified that file)
> > a subsequent re-run of the make install command would not rebuild the
> > .config file leading to either build failures or an incorrect build.
> > This change fixes that issue.
> 
> Your patch is reverting this one:
> 	mk: in install rule, don't overwrite .config if it already exists
> 	http://dpdk.org/browse/dpdk/commit/?id=1c858a7dfebd4e4092eb55
> As stated in the commit log,
> 	"This allows the user to prepare a configuration with make config
> 	before using make install."
> 
> So your patch is introducing a regression.
> 
> I think you are describing something which is not a bug.
> If you make a modification to the configuration template, you must explicitly
> call "make config".
> 
Ok, thanks for that Thomas. 
However, I still think we have a bug here, or at least we need some discussion on the correct behaviour we expect to have. From my point of view, the new behaviour is problematic, as every time I apply a patch or do a pull, I need to do a "make uninstall" or "make config" before doing "make install" just in case something has changed in the defconfig file, even if I have changed nothing on my end. This is not expected behaviour: if a change is made to the repository, doing a rebuild  should rebuild everything which needs to be built to take account of that change.

Regards,
/Bruce

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: allow updates to build config on make install
  2014-05-14 10:51   ` Richardson, Bruce
@ 2014-05-14 11:55     ` Thomas Monjalon
  2014-05-14 12:33       ` Richardson, Bruce
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Monjalon @ 2014-05-14 11:55 UTC (permalink / raw)
  To: Richardson, Bruce; +Cc: dev

2014-05-14 10:51, Richardson, Bruce:
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Wednesday, May 14, 2014 11:34 AM
> > To: Richardson, Bruce
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] mk: allow updates to build config on make
> > install
> > 
> > Hi Bruce
> > 
> > 2014-05-14 11:22, Bruce Richardson:
> > > There was an issue with rebuilding the code following a change to one
> > > of the config files inside the "config" directory. If one did a "make
> > > install T=<target>" and then made a modification to the
> > > defconfig_<target> file (or applied a patch which modified that file)
> > > a subsequent re-run of the make install command would not rebuild the
> > > .config file leading to either build failures or an incorrect build.
> > > This change fixes that issue.
> > 
> > Your patch is reverting this one:
> > 	mk: in install rule, don't overwrite .config if it already exists
> > 	http://dpdk.org/browse/dpdk/commit/?id=1c858a7dfebd4e4092eb55
> > 
> > As stated in the commit log,
> > 
> > 	"This allows the user to prepare a configuration with make config
> > 	before using make install."
> > 
> > So your patch is introducing a regression.
> > 
> > I think you are describing something which is not a bug.
> > If you make a modification to the configuration template, you must
> > explicitly call "make config".
> 
> Ok, thanks for that Thomas.
> However, I still think we have a bug here, or at least we need some
> discussion on the correct behaviour we expect to have. From my point of
> view, the new behaviour is problematic, as every time I apply a patch or do
> a pull, I need to do a "make uninstall" or "make config" before doing "make
> install" just in case something has changed in the defconfig file, even if
> I have changed nothing on my end. This is not expected behaviour: if a
> change is made to the repository, doing a rebuild  should rebuild
> everything which needs to be built to take account of that change.

You would like to have a synchronization of your generated .config file with 
the configuration template. But it's not possible for the simple reason that 
you may have modified your .config file so there is no simple correlation with 
the original template.

-- 
Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: allow updates to build config on make install
  2014-05-14 11:55     ` Thomas Monjalon
@ 2014-05-14 12:33       ` Richardson, Bruce
  2014-05-14 12:54         ` Thomas Monjalon
  0 siblings, 1 reply; 15+ messages in thread
From: Richardson, Bruce @ 2014-05-14 12:33 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, May 14, 2014 12:56 PM
> To: Richardson, Bruce
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] mk: allow updates to build config on make
> install
> 
> 
> You would like to have a synchronization of your generated .config file with the
> configuration template. But it's not possible for the simple reason that you may
> have modified your .config file so there is no simple correlation with the original
> template.
> 

Would you agree then that the ideal state matrix for a make install should probably be:
* template unmodified, config unmodified - No issue - regenerate or not as best suits the code
* template unmodified, config modified - don't regenerate, keep local config
* template modified, config modified - flag an error, since continuing compile with old config on new code will lead to undefined build results, since config template changes rarely occur without code changes to go with them.
* template modified, config unmodified - regenerate new config, overwriting old one, for the same reason above. [Optionally print out message stating that the config is being regenerated]

Does this seem reasonable to you? The last case is the common case for me in development, as I've had multiple build errors and unexpected build results in the last week alone due to config template changes not propagating as I switch development branches to work on different features.

/Bruce

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: allow updates to build config on make install
  2014-05-14 12:33       ` Richardson, Bruce
@ 2014-05-14 12:54         ` Thomas Monjalon
  2014-05-14 12:57           ` Richardson, Bruce
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Monjalon @ 2014-05-14 12:54 UTC (permalink / raw)
  To: Richardson, Bruce; +Cc: dev

2014-05-14 12:33, Richardson, Bruce:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 
> > You would like to have a synchronization of your generated .config file
> > with the configuration template. But it's not possible for the simple
> > reason that you may have modified your .config file so there is no simple
> > correlation with the original template.
> 
> Would you agree then that the ideal state matrix for a make install should
> probably be: * template unmodified, config unmodified - No issue -
> regenerate or not as best suits the code * template unmodified, config
> modified - don't regenerate, keep local config * template modified, config
> modified - flag an error, since continuing compile with old config on new
> code will lead to undefined build results, since config template changes
> rarely occur without code changes to go with them. * template modified,
> config unmodified - regenerate new config, overwriting old one, for the
> same reason above. [Optionally print out message stating that the config is
> being regenerated]
> 
> Does this seem reasonable to you? The last case is the common case for me in
> development, as I've had multiple build errors and unexpected build results
> in the last week alone due to config template changes not propagating as I
> switch development branches to work on different features.

It seems reasonable.
So you plan to send a v2 with this algorithm?

-- 
Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: allow updates to build config on make install
  2014-05-14 12:54         ` Thomas Monjalon
@ 2014-05-14 12:57           ` Richardson, Bruce
  0 siblings, 0 replies; 15+ messages in thread
From: Richardson, Bruce @ 2014-05-14 12:57 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev


> 
> It seems reasonable.
> So you plan to send a v2 with this algorithm?
> 

I haven't started investigating such a solution just yet, but I'll see what I can come up with and I'll let you know.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH v2] mk: allow updates to build config on make install
  2014-05-14 10:22 [dpdk-dev] [PATCH] mk: allow updates to build config on make install Bruce Richardson
  2014-05-14 10:33 ` Thomas Monjalon
@ 2014-05-14 15:55 ` Bruce Richardson
  2014-05-20 11:37   ` Olivier MATZ
  1 sibling, 1 reply; 15+ messages in thread
From: Bruce Richardson @ 2014-05-14 15:55 UTC (permalink / raw)
  To: dev

When running "make config" and additional config.orig file is also
generated, which is intended to hold the original, clean configuration
from the template.
When running make install, we first check if there is no existing
.config file, and run make config if not. If there is a file, we then
check if it's unmodified, in which case we regenerate a new .config to
take account of any possible updates to the template. Finally, in the
case where there is an existing .config file, and it HAS been modified,
we then do a check to see if the template has had further updates, and
throw an error if so. If no updates, we continue with the build using
the existing, user-modified config.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 mk/rte.sdkconfig.mk  |    1 +
 mk/rte.sdkinstall.mk |   10 ++++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index d0692e7..3124dce 100644
--- a/mk/rte.sdkconfig.mk
+++ b/mk/rte.sdkconfig.mk
@@ -74,6 +74,7 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE
 		-o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \
 		if ! cmp -s $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config; then \
 			cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config ; \
+			cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config.orig ; \
 		fi ; \
 		rm -f $(RTE_OUTPUT)/.config_tmp ; \
 	else \
diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index 61a8771..25ebf6b 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -54,10 +54,20 @@ INSTALL_TARGETS := $(addsuffix _install,\
 .PHONY: install
 install: $(INSTALL_TARGETS)
 
+
 %_install:
 	@echo ================== Installing $*
 	$(Q)if [ ! -f $(BUILD_DIR)/$*/.config ]; then \
 		$(MAKE) config T=$* O=$(BUILD_DIR)/$*; \
+	elif cmp -s $(BUILD_DIR)/$*/.config.orig $(BUILD_DIR)/$*/.config; then \
+		$(MAKE) config T=$* O=$(BUILD_DIR)/$*; \
+	else \
+		$(MAKE) config T=$* O=/tmp/$*; \
+		if  ! cmp -s /tmp/$*/.config.orig $(BUILD_DIR)/$*/.config.orig ; then \
+			echo "Config Conflict: Local config and standard config have both changed" ; \
+			exit 1; \
+		fi ; \
+		echo "Using local configuration" ; \
 	fi
 	$(Q)$(MAKE) all O=$(BUILD_DIR)/$*
 
-- 
1.7.0.7

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: allow updates to build config on make install
  2014-05-14 15:55 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
@ 2014-05-20 11:37   ` Olivier MATZ
  2014-06-10 13:51     ` [dpdk-dev] [PATCH v3] " Thomas Monjalon
  0 siblings, 1 reply; 15+ messages in thread
From: Olivier MATZ @ 2014-05-20 11:37 UTC (permalink / raw)
  To: Bruce Richardson, dev

Hi Bruce,

On 05/14/2014 05:55 PM, Bruce Richardson wrote:
> When running "make config" and additional config.orig file is also
> generated, which is intended to hold the original, clean configuration
> from the template.
> When running make install, we first check if there is no existing
> .config file, and run make config if not. If there is a file, we then
> check if it's unmodified, in which case we regenerate a new .config to
> take account of any possible updates to the template. Finally, in the
> case where there is an existing .config file, and it HAS been modified,
> we then do a check to see if the template has had further updates, and
> throw an error if so. If no updates, we continue with the build using
> the existing, user-modified config.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

It looks good to me, except the small typos below.

> --- a/mk/rte.sdkinstall.mk
> +++ b/mk/rte.sdkinstall.mk
> @@ -54,10 +54,20 @@ INSTALL_TARGETS := $(addsuffix _install,\
>   .PHONY: install
>   install: $(INSTALL_TARGETS)
>
> +
>   %_install:
>   	@echo ================== Installing $*
>   	$(Q)if [ ! -f $(BUILD_DIR)/$*/.config ]; then \
>   		$(MAKE) config T=$* O=$(BUILD_DIR)/$*; \

No need to add an extra line here.

> +	elif cmp -s $(BUILD_DIR)/$*/.config.orig $(BUILD_DIR)/$*/.config; then \
> +		$(MAKE) config T=$* O=$(BUILD_DIR)/$*; \
> +	else \
> +		$(MAKE) config T=$* O=/tmp/$*; \
> +		if  ! cmp -s /tmp/$*/.config.orig $(BUILD_DIR)/$*/.config.orig ; then \
> +			echo "Config Conflict: Local config and standard config have both changed" ; \
> +			exit 1; \

I would remove extra upper-case chars and say "template" instead
of "standard". Like that:

   echo "Config conflict: local config and template config have both
changed"

Apart from these minor changes,
Acked-by: Olivier Matz <olivier.matz@6wind.com>


Regards,
Olivier

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH v3] mk: allow updates to build config on make install
  2014-05-20 11:37   ` Olivier MATZ
@ 2014-06-10 13:51     ` Thomas Monjalon
  2014-06-10 16:02       ` Olivier MATZ
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Monjalon @ 2014-06-10 13:51 UTC (permalink / raw)
  To: dev

From: Bruce Richardson <bruce.richardson@intel.com>

When running "make config", an additional config.orig file is also
generated, which is intended to hold the original, clean configuration
from the template.
When running make install, we first check if there is no existing
.config file, and run make config if not. If there is a file, we then
check if it's unmodified, in which case we regenerate a new .config to
take account of any possible updates to the template. Finally, in the
case where there is an existing .config file, and it HAS been modified,
we then do a check to see if the template has had further updates, and
throw an error if so. If no updates, we continue with the build using
the existing, user-modified config.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 mk/rte.sdkconfig.mk  |  1 +
 mk/rte.sdkinstall.mk | 13 +++++++++++++
 2 files changed, 14 insertions(+)

changes in v3:
- typos commented by Olivier
- compatibility with old builds without .orig
- prefix and suffix in tmp path
- remove tmp directory if no conflict

diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index d0692e7..3124dce 100644
--- a/mk/rte.sdkconfig.mk
+++ b/mk/rte.sdkconfig.mk
@@ -74,6 +74,7 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE
 		-o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \
 		if ! cmp -s $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config; then \
 			cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config ; \
+			cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config.orig ; \
 		fi ; \
 		rm -f $(RTE_OUTPUT)/.config_tmp ; \
 	else \
diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index 24b60cf..28eb662 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -58,6 +58,19 @@ install: $(INSTALL_TARGETS)
 	@echo ================== Installing $*
 	$(Q)if [ ! -f $(BUILD_DIR)/$*/.config ]; then \
 		$(MAKE) config T=$* O=$(BUILD_DIR)/$*; \
+	elif cmp -s $(BUILD_DIR)/$*/.config.orig $(BUILD_DIR)/$*/.config; then \
+		$(MAKE) config T=$* O=$(BUILD_DIR)/$*; \
+	else \
+		if [ -f $(BUILD_DIR)/$*/.config.orig ] ; then \
+			tmp_build=/tmp/dpdk-$*-$$$$; \
+			$(MAKE) config T=$* O=$$(tmp_build); \
+			if ! cmp -s $(BUILD_DIR)/$*/.config.orig $$(tmp_build)/.config ; then \
+				echo "Conflict: local config and template config have both changed"; \
+				exit 1; \
+			fi; \
+			rm -rf $$(tmp_build); \
+		fi; \
+		echo "Using local configuration"; \
 	fi
 	$(Q)$(MAKE) all O=$(BUILD_DIR)/$*
 
-- 
2.0.0

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH v3] mk: allow updates to build config on make install
  2014-06-10 13:51     ` [dpdk-dev] [PATCH v3] " Thomas Monjalon
@ 2014-06-10 16:02       ` Olivier MATZ
  2014-06-10 16:29         ` Richardson, Bruce
  0 siblings, 1 reply; 15+ messages in thread
From: Olivier MATZ @ 2014-06-10 16:02 UTC (permalink / raw)
  To: Thomas Monjalon, dev

Hi Thomas,

On 06/10/2014 03:51 PM, Thomas Monjalon wrote:
> +		if [ -f $(BUILD_DIR)/$*/.config.orig ] ; then \
> +			tmp_build=/tmp/dpdk-$*-$$$$; \
> +			$(MAKE) config T=$* O=$$(tmp_build); \
> +			if ! cmp -s $(BUILD_DIR)/$*/.config.orig $$(tmp_build)/.config ; then \
> +				echo "Conflict: local config and template config have both changed"; \
> +				exit 1; \

I missed it the first time, but what do you think about using
$(BUILD_DIR)/$*/.config.tmp instead of /tmp/dpdk-$*-$$ ?
I think using /tmp should be avoided when possible as it is a
shared folder.

Regards,
Olivier

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH v3] mk: allow updates to build config on make install
  2014-06-10 16:02       ` Olivier MATZ
@ 2014-06-10 16:29         ` Richardson, Bruce
  2014-06-10 16:38           ` [dpdk-dev] [PATCH v4] " Thomas Monjalon
  0 siblings, 1 reply; 15+ messages in thread
From: Richardson, Bruce @ 2014-06-10 16:29 UTC (permalink / raw)
  To: Olivier MATZ, Thomas Monjalon, dev

Yes, good point. I'll try and redraft a v3 of the patch (thanks Thomas for doing a V2), with that in it. I also think if we keep everything in the build dir we should not the rm afterwards. [In my previous tests when doing V1 patch, I found that deleting the directory each time seemed to slow things down for the next run. I'll test if that is still the case when doing V3]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ
> Sent: Tuesday, June 10, 2014 9:03 AM
> To: Thomas Monjalon; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] mk: allow updates to build config on make
> install
> 
> Hi Thomas,
> 
> On 06/10/2014 03:51 PM, Thomas Monjalon wrote:
> > +		if [ -f $(BUILD_DIR)/$*/.config.orig ] ; then \
> > +			tmp_build=/tmp/dpdk-$*-$$$$; \
> > +			$(MAKE) config T=$* O=$$(tmp_build); \
> > +			if ! cmp -s $(BUILD_DIR)/$*/.config.orig
> $$(tmp_build)/.config ; then \
> > +				echo "Conflict: local config and template
> config have both changed"; \
> > +				exit 1; \
> 
> I missed it the first time, but what do you think about using
> $(BUILD_DIR)/$*/.config.tmp instead of /tmp/dpdk-$*-$$ ?
> I think using /tmp should be avoided when possible as it is a
> shared folder.
> 
> Regards,
> Olivier

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH v4] mk: allow updates to build config on make install
  2014-06-10 16:29         ` Richardson, Bruce
@ 2014-06-10 16:38           ` Thomas Monjalon
  2014-06-10 18:43             ` Richardson, Bruce
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Monjalon @ 2014-06-10 16:38 UTC (permalink / raw)
  To: dev

From: Bruce Richardson <bruce.richardson@intel.com>

When running "make config", an additional config.orig file is also
generated, which is intended to hold the original, clean configuration
from the template.
When running make install, we first check if there is no existing
.config file, and run make config if not. If there is a file, we then
check if it's unmodified, in which case we regenerate a new .config to
take account of any possible updates to the template. Finally, in the
case where there is an existing .config file, and it HAS been modified,
we then do a check to see if the template has had further updates, and
throw an error if so. If no updates, we continue with the build using
the existing, user-modified config.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 mk/rte.sdkconfig.mk  |  1 +
 mk/rte.sdkinstall.mk | 12 ++++++++++++
 2 files changed, 13 insertions(+)

changes in v3:
- typos commented by Olivier
- compatibility with old builds without .orig
- prefix and suffix in tmp path
- remove tmp directory if no conflict

changes in v4:
- tmp directory in build directory
- do not remove tmp directory to speed up rebuild

diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index d0692e7..3124dce 100644
--- a/mk/rte.sdkconfig.mk
+++ b/mk/rte.sdkconfig.mk
@@ -74,6 +74,7 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE
 		-o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \
 		if ! cmp -s $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config; then \
 			cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config ; \
+			cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config.orig ; \
 		fi ; \
 		rm -f $(RTE_OUTPUT)/.config_tmp ; \
 	else \
diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index 24b60cf..8571196 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -58,6 +58,18 @@ install: $(INSTALL_TARGETS)
 	@echo ================== Installing $*
 	$(Q)if [ ! -f $(BUILD_DIR)/$*/.config ]; then \
 		$(MAKE) config T=$* O=$(BUILD_DIR)/$*; \
+	elif cmp -s $(BUILD_DIR)/$*/.config.orig $(BUILD_DIR)/$*/.config; then \
+		$(MAKE) config T=$* O=$(BUILD_DIR)/$*; \
+	else \
+		if [ -f $(BUILD_DIR)/$*/.config.orig ] ; then \
+			tmp_build=$(BUILD_DIR)/$*/.config.tmp; \
+			$(MAKE) config T=$* O=$$tmp_build; \
+			if ! cmp -s $(BUILD_DIR)/$*/.config.orig $$tmp_build/.config ; then \
+				echo "Conflict: local config and template config have both changed"; \
+				exit 1; \
+			fi; \
+		fi; \
+		echo "Using local configuration"; \
 	fi
 	$(Q)$(MAKE) all O=$(BUILD_DIR)/$*
 
-- 
2.0.0

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH v4] mk: allow updates to build config on make install
  2014-06-10 16:38           ` [dpdk-dev] [PATCH v4] " Thomas Monjalon
@ 2014-06-10 18:43             ` Richardson, Bruce
  2014-06-11  9:54               ` Thomas Monjalon
  0 siblings, 1 reply; 15+ messages in thread
From: Richardson, Bruce @ 2014-06-10 18:43 UTC (permalink / raw)
  To: Thomas Monjalon, dev



> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, June 10, 2014 9:38 AM
> To: dev@dpdk.org
> Cc: Richardson, Bruce
> Subject: [PATCH v4] mk: allow updates to build config on make install
> 
> From: Bruce Richardson <bruce.richardson@intel.com>
> 
> When running "make config", an additional config.orig file is also
> generated, which is intended to hold the original, clean configuration
> from the template.
> When running make install, we first check if there is no existing
> .config file, and run make config if not. If there is a file, we then
> check if it's unmodified, in which case we regenerate a new .config to
> take account of any possible updates to the template. Finally, in the
> case where there is an existing .config file, and it HAS been modified,
> we then do a check to see if the template has had further updates, and
> throw an error if so. If no updates, we continue with the build using
> the existing, user-modified config.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Tested-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH v4] mk: allow updates to build config on make install
  2014-06-10 18:43             ` Richardson, Bruce
@ 2014-06-11  9:54               ` Thomas Monjalon
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Monjalon @ 2014-06-11  9:54 UTC (permalink / raw)
  To: Richardson, Bruce; +Cc: dev

> > From: Bruce Richardson <bruce.richardson@intel.com>
> > 
> > When running "make config", an additional config.orig file is also
> > generated, which is intended to hold the original, clean configuration
> > from the template.
> > When running make install, we first check if there is no existing
> > .config file, and run make config if not. If there is a file, we then
> > check if it's unmodified, in which case we regenerate a new .config to
> > take account of any possible updates to the template. Finally, in the
> > case where there is an existing .config file, and it HAS been modified,
> > we then do a check to see if the template has had further updates, and
> > throw an error if so. If no updates, we continue with the build using
> > the existing, user-modified config.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> Tested-by: Bruce Richardson <bruce.richardson@intel.com>

Previous version was acked by Olivier.

It is now applied for version 1.7.0.

-- 
Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-06-11  9:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-14 10:22 [dpdk-dev] [PATCH] mk: allow updates to build config on make install Bruce Richardson
2014-05-14 10:33 ` Thomas Monjalon
2014-05-14 10:51   ` Richardson, Bruce
2014-05-14 11:55     ` Thomas Monjalon
2014-05-14 12:33       ` Richardson, Bruce
2014-05-14 12:54         ` Thomas Monjalon
2014-05-14 12:57           ` Richardson, Bruce
2014-05-14 15:55 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
2014-05-20 11:37   ` Olivier MATZ
2014-06-10 13:51     ` [dpdk-dev] [PATCH v3] " Thomas Monjalon
2014-06-10 16:02       ` Olivier MATZ
2014-06-10 16:29         ` Richardson, Bruce
2014-06-10 16:38           ` [dpdk-dev] [PATCH v4] " Thomas Monjalon
2014-06-10 18:43             ` Richardson, Bruce
2014-06-11  9:54               ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).