* [PATCH] i2c-tools: lib/Module.mk: fixing LIB_LINKS dependency @ 2017-12-11 17:00 Angelo Compagnucci 2017-12-14 8:36 ` Jean Delvare 0 siblings, 1 reply; 4+ messages in thread From: Angelo Compagnucci @ 2017-12-11 17:00 UTC (permalink / raw) To: linux-i2c; +Cc: Angelo Compagnucci LIB_LINKS should be added as a dependency only when BUILD_DYNAMIC_LIB is enabled Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> --- lib/Module.mk | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Module.mk b/lib/Module.mk index 8a58f5b..de189a2 100644 --- a/lib/Module.mk +++ b/lib/Module.mk @@ -27,10 +27,9 @@ LIB_SHSONAME := $(LIB_SHBASENAME).$(LIB_MAINVER) LIB_SHLIBNAME := $(LIB_SHBASENAME).$(LIB_VER) LIB_STLIBNAME := libi2c.a -LIB_LINKS := $(LIB_SHSONAME) $(LIB_SHBASENAME) - LIB_TARGETS := ifeq ($(BUILD_DYNAMIC_LIB),1) +LIB_LINKS := $(LIB_SHSONAME) $(LIB_SHBASENAME) LIB_TARGETS += $(LIB_SHLIBNAME) endif ifeq ($(BUILD_STATIC_LIB),1) -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: i2c-tools: lib/Module.mk: fixing LIB_LINKS dependency 2017-12-11 17:00 [PATCH] i2c-tools: lib/Module.mk: fixing LIB_LINKS dependency Angelo Compagnucci @ 2017-12-14 8:36 ` Jean Delvare 2017-12-14 9:43 ` Angelo Compagnucci 0 siblings, 1 reply; 4+ messages in thread From: Jean Delvare @ 2017-12-14 8:36 UTC (permalink / raw) To: Angelo Compagnucci; +Cc: linux-i2c Hi Angelo, On Mon, 11 Dec 2017 18:00:24 +0100, Angelo Compagnucci wrote: > LIB_LINKS should be added as a dependency only when > BUILD_DYNAMIC_LIB is enabled > > Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> > --- > lib/Module.mk | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/lib/Module.mk b/lib/Module.mk > index 8a58f5b..de189a2 100644 > --- a/lib/Module.mk > +++ b/lib/Module.mk > @@ -27,10 +27,9 @@ LIB_SHSONAME := $(LIB_SHBASENAME).$(LIB_MAINVER) > LIB_SHLIBNAME := $(LIB_SHBASENAME).$(LIB_VER) > LIB_STLIBNAME := libi2c.a > > -LIB_LINKS := $(LIB_SHSONAME) $(LIB_SHBASENAME) > - > LIB_TARGETS := > ifeq ($(BUILD_DYNAMIC_LIB),1) > +LIB_LINKS := $(LIB_SHSONAME) $(LIB_SHBASENAME) > LIB_TARGETS += $(LIB_SHLIBNAME) > endif > ifeq ($(BUILD_STATIC_LIB),1) This is correct, good catch. I'll commit it. In fact I thought about it when reviewing your previous patch, but (wrongly) assumed it did not matter. After adding all the missing dependencies, it clearly does. I have noticed another problem. While "make BUILD_STATIC_LIB=0" actually prevents the static flavor of the library from being built, "make BUILD_DYNAMIC_LIB=0" does not prevent the dynamic flavor of the library from being built. The reason is that USE_STATIC_LIB is not set, so tools depend on the dynamic library, which in turn gets built as a dependency. I suppose that BUILD_DYNAMIC_LIB=0 should imply USE_STATIC_LIB=1. Something like: --- a/Makefile +++ b/Makefile @@ -43,6 +43,8 @@ endif ifeq ($(BUILD_DYNAMIC_LIB),0) ifeq ($(BUILD_STATIC_LIB),0) $(error BUILD_DYNAMIC_LIB and BUILD_STATIC_LIB cannot be disabled at the same time) +else +USE_STATIC_LIB := 1 endif endif What do you think? Thanks, -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: i2c-tools: lib/Module.mk: fixing LIB_LINKS dependency 2017-12-14 8:36 ` Jean Delvare @ 2017-12-14 9:43 ` Angelo Compagnucci 2017-12-14 12:42 ` Jean Delvare 0 siblings, 1 reply; 4+ messages in thread From: Angelo Compagnucci @ 2017-12-14 9:43 UTC (permalink / raw) To: Jean Delvare; +Cc: Linux I2C DEar Jean, On Thu, Dec 14, 2017 at 9:36 AM, Jean Delvare <jdelvare@suse.de> wrote: > Hi Angelo, > > On Mon, 11 Dec 2017 18:00:24 +0100, Angelo Compagnucci wrote: >> LIB_LINKS should be added as a dependency only when >> BUILD_DYNAMIC_LIB is enabled >> >> Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> >> --- >> lib/Module.mk | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/lib/Module.mk b/lib/Module.mk >> index 8a58f5b..de189a2 100644 >> --- a/lib/Module.mk >> +++ b/lib/Module.mk >> @@ -27,10 +27,9 @@ LIB_SHSONAME := $(LIB_SHBASENAME).$(LIB_MAINVER) >> LIB_SHLIBNAME := $(LIB_SHBASENAME).$(LIB_VER) >> LIB_STLIBNAME := libi2c.a >> >> -LIB_LINKS := $(LIB_SHSONAME) $(LIB_SHBASENAME) >> - >> LIB_TARGETS := >> ifeq ($(BUILD_DYNAMIC_LIB),1) >> +LIB_LINKS := $(LIB_SHSONAME) $(LIB_SHBASENAME) >> LIB_TARGETS += $(LIB_SHLIBNAME) >> endif >> ifeq ($(BUILD_STATIC_LIB),1) > > This is correct, good catch. I'll commit it. In fact I thought about it > when reviewing your previous patch, but (wrongly) assumed it did not > matter. After adding all the missing dependencies, it clearly does. > > I have noticed another problem. While "make BUILD_STATIC_LIB=0" > actually prevents the static flavor of the library from being built, > "make BUILD_DYNAMIC_LIB=0" does not prevent the dynamic flavor of the > library from being built. The reason is that USE_STATIC_LIB is not set, > so tools depend on the dynamic library, which in turn gets built as a > dependency. > > I suppose that BUILD_DYNAMIC_LIB=0 should imply USE_STATIC_LIB=1. > Something like: > > --- a/Makefile > +++ b/Makefile > @@ -43,6 +43,8 @@ endif > ifeq ($(BUILD_DYNAMIC_LIB),0) > ifeq ($(BUILD_STATIC_LIB),0) > $(error BUILD_DYNAMIC_LIB and BUILD_STATIC_LIB cannot be disabled at the same time) > +else > +USE_STATIC_LIB := 1 > endif > endif > > What do you think? I think you are totally right. If BUILD_DYNAMIC_LIB is 0 the static variant should be built and used too. Could you realease a new version of i2c-tools with these patches? I'm in the phase of updating the package in buildroot and I do really prefer to have a new stable release than an old release with a bunch of patches on top. Thanks! > > Thanks, > -- > Jean Delvare > SUSE L3 Support ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: i2c-tools: lib/Module.mk: fixing LIB_LINKS dependency 2017-12-14 9:43 ` Angelo Compagnucci @ 2017-12-14 12:42 ` Jean Delvare 0 siblings, 0 replies; 4+ messages in thread From: Jean Delvare @ 2017-12-14 12:42 UTC (permalink / raw) To: Angelo Compagnucci; +Cc: Linux I2C On Thu, 14 Dec 2017 10:43:33 +0100, Angelo Compagnucci wrote: > On Thu, Dec 14, 2017 at 9:36 AM, Jean Delvare <jdelvare@suse.de> wrote: > > I suppose that BUILD_DYNAMIC_LIB=0 should imply USE_STATIC_LIB=1. > > Something like: > > > > --- a/Makefile > > +++ b/Makefile > > @@ -43,6 +43,8 @@ endif > > ifeq ($(BUILD_DYNAMIC_LIB),0) > > ifeq ($(BUILD_STATIC_LIB),0) > > $(error BUILD_DYNAMIC_LIB and BUILD_STATIC_LIB cannot be disabled at the same time) > > +else > > +USE_STATIC_LIB := 1 > > endif > > endif > > > > What do you think? > > I think you are totally right. If BUILD_DYNAMIC_LIB is 0 the static > variant should be built and used too. Thanks. Both fixes committed. > Could you realease a new version of i2c-tools with these patches? I'm > in the phase of updating the package in buildroot and I do really > prefer to have a new stable release than an old release with a bunch > of patches on top. I don't have such a plan, sorry. Version 4.0 is still relatively new, I expect more problems to be found and fixed in the next weeks, I don't think it makes sense to rush a new release. There are only 13 commits since 4.0, it's not that many. Backporting patches for distribution packages is the usual process, and 6 patches is not a bunch. If the patch count really bothers you, you can always consolidate the fix-ups into the original patch and you'll be down to 4, that seems easily manageable. Quilt is your friend ;-) -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-12-14 12:42 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-11 17:00 [PATCH] i2c-tools: lib/Module.mk: fixing LIB_LINKS dependency Angelo Compagnucci 2017-12-14 8:36 ` Jean Delvare 2017-12-14 9:43 ` Angelo Compagnucci 2017-12-14 12:42 ` Jean Delvare
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox