Message ID | 1513011624-1555-1-git-send-email-angelo@amarulasolutions.com |
---|---|
State | Awaiting Upstream |
Headers | show |
Series | i2c-tools: lib/Module.mk: fixing LIB_LINKS dependency | expand |
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,
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
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 ;-)
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)
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(-)