Message ID | 1471939333-956-1-git-send-email-ash.benz@bk.ru |
---|---|
State | Changes Requested |
Headers | show |
On 2016-08-23 10:02, Ash Benz wrote: > On some systems the shell built-in echo is used while on others /bin/echo and those two are not > identical; this causes incomplete .cflags generation on some systems. > > Signed-off-by: Ash Benz <ash.benz@bk.ru> > --- > package/network/services/hostapd/patches/200-multicall.patch | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/package/network/services/hostapd/patches/200-multicall.patch b/package/network/services/hostapd/patches/200-multicall.patch > index 8b260c2..ddfec6e 100644 > --- a/package/network/services/hostapd/patches/200-multicall.patch > +++ b/package/network/services/hostapd/patches/200-multicall.patch > @@ -54,10 +54,10 @@ > endif > > +dump_cflags: > -+ @echo -n $(CFLAGS) " " > ++ @printf "%s " $(CFLAGS) > + > +dump_ldflags: > -+ @echo -n $(LDFLAGS) $(LIBS) $(EXTRALIBS) " " > ++ @printf "%s " $(LDFLAGS) $(LIBS) $(EXTRALIBS) > + > nt_password_hash: $(NOBJS) > $(Q)$(CC) $(LDFLAGS) -o nt_password_hash $(NOBJS) $(LIBS_n) > @@ -142,10 +142,10 @@ > @$(E) " sed" $< > > +dump_cflags: > -+ @echo -n $(CFLAGS) " " > ++ @printf "%s " $(CFLAGS) > + > +dump_ldflags: > -+ @echo -n $(LDFLAGS) $(LIBS) $(EXTRALIBS) " " > ++ @printf "%s " $(LDFLAGS) $(LIBS) $(EXTRALIBS) While your version appears to work, I think it would be more correct to put everything in one argument for %s. Please use something like: @printf "%s " "$(LDFLAGS) $(LIBS) $(EXTRALIBS)" - Felix
On 23 August 2016 at 10:02, Ash Benz <ash.benz@bk.ru> wrote: > On some systems the shell built-in echo is used while on others /bin/echo and those two are not > identical; this causes incomplete .cflags generation on some systems. > > Signed-off-by: Ash Benz <ash.benz@bk.ru> Please use a proper prefix for your subject. For a good hint see git log --oneline package/network/services/hostapd/ Also no need to prepend every line of your commit message with a space. Just write "On some" instead of " On some".
Hi Bastian, I will update them, however to do that while adhering to Felix's request on using perfect syntax means going over them one by one. Perhaps a seasoned sed user can do a trick to automate it? For now, the change I submitted is whats needed to fix breakage on my system. For others, I will send patches if no one takes it up in a few days. Regards, A. Benz On 08/23/16 17:03, Bastian Bittorf wrote: > * Ash Benz <ash.benz@bk.ru> [23.08.2016 10:19]: >> On some systems the shell built-in echo is used while on others /bin/echo and those two are not >> identical; this causes incomplete .cflags generation on some systems. > > thanks for that, it was on my list. > can you also eleminate some other users, see: > > # git grep 'echo -n' | wc -l > # 87 > > thanks & bye, bastian >
On 2016-08-23 12:10, A. Benz wrote: > Hi Bastian, > > I will update them, however to do that while adhering to Felix's request > on using perfect syntax means going over them one by one. > > Perhaps a seasoned sed user can do a trick to automate it? > > For now, the change I submitted is whats needed to fix breakage on my > system. For others, I will send patches if no one takes it up in a few days. Actually, you might be able to simplify the change even more by forcing the hostapd build shell to bash (some other packages do this as well). - Felix
* Felix Fietkau <nbd@nbd.name> [23.08.2016 13:59]: > > For now, the change I submitted is whats needed to fix breakage on my > > system. For others, I will send patches if no one takes it up in a few days. > Actually, you might be able to simplify the change even more by forcing > the hostapd build shell to bash (some other packages do this as well). this look simple, but in the long run... # "Stick to portable constructs where possible, and # you will make somebody's life easier in the future. # Maybe your own." so using /bin/sh with 'printf' is safe. (no matter which underlying shell-interpreter we use) bye, bastian
Felix Fietkau <nbd@nbd.name> wrote: > On 2016-08-23 12:10, A. Benz wrote: > > Hi Bastian, > > > > I will update them, however to do that while adhering to Felix's request > > on using perfect syntax means going over them one by one. > > > > Perhaps a seasoned sed user can do a trick to automate it? > > > > For now, the change I submitted is whats needed to fix breakage on my > > system. For others, I will send patches if no one takes it up in a few days. > Actually, you might be able to simplify the change even more by > forcing the hostapd build shell to bash (some other packages do > this as well). really? Surely printf is better than requiring bash?
diff --git a/package/network/services/hostapd/patches/200-multicall.patch b/package/network/services/hostapd/patches/200-multicall.patch index 8b260c2..ddfec6e 100644 --- a/package/network/services/hostapd/patches/200-multicall.patch +++ b/package/network/services/hostapd/patches/200-multicall.patch @@ -54,10 +54,10 @@ endif +dump_cflags: -+ @echo -n $(CFLAGS) " " ++ @printf "%s " $(CFLAGS) + +dump_ldflags: -+ @echo -n $(LDFLAGS) $(LIBS) $(EXTRALIBS) " " ++ @printf "%s " $(LDFLAGS) $(LIBS) $(EXTRALIBS) + nt_password_hash: $(NOBJS) $(Q)$(CC) $(LDFLAGS) -o nt_password_hash $(NOBJS) $(LIBS_n) @@ -142,10 +142,10 @@ @$(E) " sed" $< +dump_cflags: -+ @echo -n $(CFLAGS) " " ++ @printf "%s " $(CFLAGS) + +dump_ldflags: -+ @echo -n $(LDFLAGS) $(LIBS) $(EXTRALIBS) " " ++ @printf "%s " $(LDFLAGS) $(LIBS) $(EXTRALIBS) + wpa_supplicant.exe: wpa_supplicant mv -f $< $@
On some systems the shell built-in echo is used while on others /bin/echo and those two are not identical; this causes incomplete .cflags generation on some systems. Signed-off-by: Ash Benz <ash.benz@bk.ru> --- package/network/services/hostapd/patches/200-multicall.patch | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)