new <pkg>_CONFIG_FIXUP variable
This patch provides new variable called <pkg>_CONFIG_FIXUP.
It's purpose is to fix wrong prefixes of $(STAGING_DIR)/usr/bin/*-config file(s) that many library packages install after staging installation phase.
Some packages provide multible *-config files.
So as suggested by Thomas Petazzoni and Gustavo Zacarias this variable can be feeded multible files.
All filenames are relative to $(STAGING_DIR)/usr/bin
Example:
DIVINE_CONFIG_FIXUP = divine-config
or:
IMAGEMAGICK_CONFIG_FIXUP = Magick-config Wand-config
Regards
Stefan
PATCH 1/1 new variable <pkg>_CONFIG_FIXUP
--- package/pkg-generic.mk | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
package/pkg-generic.mk +8 -0
@@ -121,6 +121,14 @@ $(BUILD_DIR)/%/.stamp_staging_installed: @$(call MESSAGE,"Installing to staging directory") $($(PKG)_INSTALL_STAGING_CMDS) $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) + @$(call MESSAGE,"Fixing package configuration files") + for file in $($(PKG)_CONFIG_FIXUP); do \ + if [ -e $(STAGING_DIR)/usr/bin/$${file} ]; then \ + $(SED) "s,^prefix=.*,prefix=\'$(STAGING_DIR)/usr\',g" \ + -e "s,^exec_prefix=.*,exec_prefix=\'$(STAGING_DIR)/usr\',g" \ + $(STAGING_DIR)/usr/bin/$${file} ;\ + fi \ + done $(Q)touch $@ # Install to images dir
Signature · 1 line
1.7.7.6
PATCH 1/1 new variable <pkg>_CONFIG_FIXUP
Dear Stefan Fr?berg,
Quoted 1 line · Stefan Fr?berg
Signed-off-by Stefan Fr?berg <[email protected]>
Please add more details to your commit log. I.e, basically, most of your introduction e-mail contents should go here.
Remember: the contents of your introduction e-mail are not preserved in the Git history. On the opposite, the commit log is preserved forever in the Git history, so we want the details to be here.
package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_staging_installed: · line 124
Quoted 13 lines
--- package/pkg-generic.mk | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)package/pkg-generic.mk +1
-0@@ -121,6 +121,14 @@ $(BUILD_DIR)/%/.stamp_staging_installed: @$(call MESSAGE,"Installing to staging directory") $($(PKG)_INSTALL_STAGING_CMDS) $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) + @$(call MESSAGE,"Fixing package configuration files")
Please show the message only if there are actually files to fixup. Or maybe just don't display any message at all. Fixing those files is kind of a detail.
package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_staging_installed: · line 126
Quoted 2 lines
package/pkg-generic.mk +2
-0+ for file in $($(PKG)_CONFIG_FIXUP); do \ + if [ -e $(STAGING_DIR)/usr/bin/$${file} ]; then \
No, we should not silently error out if the file doesn't exist, because it's hiding a mistake of package .mk file. Instead, if the file doesn't exist, we should loudly error out.
Also, please include an update to the Buildroot manual, either as part of this patch, or as a separate patch.
Thanks for your work!
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 1/1 new variable <pkg>_CONFIG_FIXUP
Hi Thomas
8.1.2013 0:10, Thomas Petazzoni kirjoitti:
Quoted 6 lines
Dear Stefan Fr?berg,
Quoted 1 line · Stefan Fr?berg
Signed-off-by Stefan Fr?berg <[email protected]>Please add more details to your commit log. I.e, basically, most of your introduction e-mail contents should go here.
Okay
Quoted 8 lines
Remember: the contents of your introduction e-mail are not preserved in the Git history. On the opposite, the commit log is preserved forever in the Git history, so we want the details to be here.
Quoted 4 lines
--- package/pkg-generic.mk | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
Quoted 12 lines
package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_staging_installed: · line 124
Quoted 9 lines
package/pkg-generic.mk +1
-0@@ -121,6 +121,14 @@ $(BUILD_DIR)/%/.stamp_staging_installed: @$(call MESSAGE,"Installing to staging directory") $($(PKG)_INSTALL_STAGING_CMDS) $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) + @$(call MESSAGE,"Fixing package configuration files")Please show the message only if there are actually files to fixup. Or maybe just don't display any message at all. Fixing those files is kind of a detail.
You mean checking first in if-statement if $($(PKG)_CONFIG_FIXUP exist ? What is the best way to detect if it exist ?
Quoted 5 lines
package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_staging_installed: · line 126
Quoted 2 lines
package/pkg-generic.mk +2
-0+ for file in $($(PKG)_CONFIG_FIXUP); do \ + if [ -e $(STAGING_DIR)/usr/bin/$${file} ]; then \No, we should not silently error out if the file doesn't exist, because it's hiding a mistake of package .mk file. Instead, if the file doesn't exist, we should loudly error out.
Okay
Quoted 2 lines
Also, please include an update to the Buildroot manual, either as part of this patch, or as a separate patch.
Oh my, I really suck when doing any documentation stuff. But ill try.....
Quoted 3 lines
Thanks for your work!
Thomas
Thank you!
Stefan
PATCH 1/1 new variable <pkg>_CONFIG_FIXUP
Dear Stefan Fr?berg,
Quoted 2 lines · Stefan Fr?berg
You mean checking first in if-statement if $($(PKG)_CONFIG_FIXUP exist ? What is the best way to detect if it exist ?
Just test if the variable is not empty, i.e test -n.
Like (untested):
if test -n $($(PKG)_CONFIG_FIXUP) ; then \ $(call MESSAGE, "Blabla") ; \ for file in ... ... done fi
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 1/1 new variable <pkg>_CONFIG_FIXUP
Quoted 9 lines · Thomas Petazzoni
Dear Stefan Fr?berg,
Quoted 2 lines · Stefan Fr?berg
You mean checking first in if-statement if $($(PKG)_CONFIG_FIXUP exist ? What is the best way to detect if it exist ?
Just test if the variable is not empty, i.e test -n.
Quoted 8 lines
Like (untested):
if test -n $($(PKG)_CONFIG_FIXUP) ; then \ $(call MESSAGE, "Blabla") ; \ for file in ... ... done fi
The test isn't necessary, the loop will not execute if it is empty.
Stefan, sed will give an appropriate error if the file doesn't exist.
Regards,
Arnout
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
PATCH 1/1 new variable <pkg>_CONFIG_FIXUP
Dear Arnout Vandecappelle,
Quoted 10 lines · Arnout Vandecappelle
Quoted 8 lines
Like (untested):
if test -n $($(PKG)_CONFIG_FIXUP) ; then \ $(call MESSAGE, "Blabla") ; \ for file in ... ... done fiThe test isn't necessary, the loop will not execute if it is empty.
See the original discussion. The test is necessary to not show the message "Fixing package configuration files" if there are no files to fix, i.e if the <PKG>_CONFIG_FIXUP variable is empty.
Best regards,
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 1/1 new variable <pkg>_CONFIG_FIXUP
In reply to Stefan FröbergQuoted 10 lines · Stefan Fr?berg
Signed-off-by Stefan Fr?berg<[email protected]>--- package/pkg-generic.mk | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)package/pkg-generic.mk +0
-0
package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_staging_installed: · line 127
Quoted 7 lines
package/pkg-generic.mk +4
-0@@ -121,6 +121,14 @@ $(BUILD_DIR)/%/.stamp_staging_installed: @$(call MESSAGE,"Installing to staging directory") $($(PKG)_INSTALL_STAGING_CMDS) $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) + @$(call MESSAGE,"Fixing package configuration files") + for file in $($(PKG)_CONFIG_FIXUP); do \ + if [ -e $(STAGING_DIR)/usr/bin/$${file} ]; then \ + $(SED) "s,^prefix=.*,prefix=\'$(STAGING_DIR)/usr\',g" \
The \' are redundant, just put ' (it's already between double quotes).
package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_staging_installed: · line 131
Quoted 4 lines
package/pkg-generic.mk +4
-0+ -e "s,^exec_prefix=.*,exec_prefix=\'$(STAGING_DIR)/usr\',g" \ + $(STAGING_DIR)/usr/bin/$${file} ;\ + fi \ + done
Since sed -i is used, you can actually pass all files together.
Something like:
if [ "$($(PKG)_CONFIG_FIXUP)" ]; then \ $(SED) ... \ $(addprefix $(STAGING_DIR)/usr/bin,$($(PKG)_CONFIG_FIXUP))); \
fi
Regards,
Arnout
Quoted 3 lines
package/pkg-generic.mk +0
-0$(Q)touch $@ # Install to images dir
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
Stefan, All,
Quoted 11 lines · On Monday 07 January 2013 Stefan Fr?berg wrote:
This patch provides new variable called <pkg>_CONFIG_FIXUP.
It's purpose is to fix wrong prefixes of $(STAGING_DIR)/usr/bin/*-config file(s) that many library packages install after staging installation phase.Some packages provide multible *-config files.
So as suggested by Thomas Petazzoni and Gustavo Zacarias this variable can be feeded multible files.
All filenames are relative to $(STAGING_DIR)/usr/bin
Quoted 7 lines
Example:
DIVINE_CONFIG_FIXUP= divine-configor:
IMAGEMAGICK_CONFIG_FIXUP= Magick-config Wand-config
Please, put this explanation in the commit message.
Also, add this variable to the documentation:
docs/manual/adding-packages-generic.txt
Next time, also add a snall changelog to your message, so we know there was prior discussion and to help Peter to apply the correct patch series. I do something like:
Changes v1 -> v2:
- accept multiple files (Thomas, Gustavo)
- use paths relative to $(STAGING)/usr/bin
And keep this changelog in each iteration. Add the version of the iteration in your mail subject.
Thank you! :-)
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
Hi Yann
8.1.2013 0:10, Yann E. MORIN kirjoitti:
Quoted 33 lines
Stefan, All,
Quoted 11 lines · On Monday 07 January 2013 Stefan Fr?berg wrote:
This patch provides new variable called <pkg>_CONFIG_FIXUP.
It's purpose is to fix wrong prefixes of $(STAGING_DIR)/usr/bin/*-config file(s) that many library packages install after staging installation phase.Some packages provide multible *-config files.
So as suggested by Thomas Petazzoni and Gustavo Zacarias this variable can be feeded multible files.
All filenames are relative to $(STAGING_DIR)/usr/bin
Quoted 7 lines
Example:
DIVINE_CONFIG_FIXUP= divine-configor:
IMAGEMAGICK_CONFIG_FIXUP= Magick-config Wand-configPlease, put this explanation in the commit message.
Also, add this variable to the documentation:
docs/manual/adding-packages-generic.txtNext time, also add a snall changelog to your message, so we know there was prior discussion and to help Peter to apply the correct patch series. I do something like:
Changes v1 -> v2:
- accept multiple files (Thomas, Gustavo)
- use paths relative to $(STAGING)/usr/bin
Quoted 8 lines
And keep this changelog in each iteration. Add the version of the iteration in your mail subject.
Thank you! :-)
Regards,
Yann E. MORIN.
God...
It seems that doing documentation for all these things is going to take more work than the actual patch itself
;-)
Thanks!
Stefan
Stefan, All,
On Monday 07 January 2013 Stefan Fr?berg wrote:
[--SNIP--]
Quoted 3 lines
God...
It seems that doing documentation for all these things is going to take more work than the actual patch itself
Well, in my view, the code is actually only one fourth of the work. The other three fourth are split thus:
- 1/4 tests
- 1/4 documentation
- 1/4 the rest: commit message, discussing, regression-testing, forensic analysis...
In some cases, the code and tests are quickly done, because simple, and the doc takes 90% of the time.
Other times, the documentation is easily written, but the code is very complex to write, and the tests very hard to write and run.
Sometimes, discussing an issue is what takes the longest, for finally very simple pieces of code, test and documentation.
And it can happen that an issue is very hard to identify, and it takes a very long time to find a suitable fix.
All in all, in my experience, that's roughly split in those four fourths. Comunity work like here, and professional work alike.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
Dear Yann E. MORIN,
Quoted 2 lines · Yann E. MORIN
All in all, in my experience, that's roughly split in those four fourths. Comunity work like here, and professional work alike.
Agreed. In this specific case, I'd say about 5 lines of documentation are probably needed. I'm willing to provide the patch if needed.
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
Hi Thomas
I tried that empty variable checking example you gave but it won't work. If I remove DIVINE_CONFIG_FIXUP from divine.mk it will still spit the "Fixing package configuration files" message
This is what it looks so far. Added your variable checking and removed redundant '\ from sed line as Arnout said.
if test -n $($(PKG)_CONFIG_FIXUP) ; then \
$(call MESSAGE,"Fixing package configuration files") ;\
for file in $($(PKG)_CONFIG_FIXUP); do \
if [ -e $(STAGING_DIR)/usr/bin/$${file} ]; then \
$(SED) "s,^prefix=.*,prefix=$(STAGING_DIR)/usr,g" \
-e
"s,^exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr,g" \
$(STAGING_DIR)/usr/bin/$${file} ;\
fi \
done \
fi
$(Q)touch $@And this is what happens:
test -z "/usr/bin" || /bin/mkdir -p "/mnt/hee/new/buildroot-2012.08/output/host/usr/i586-unknown-linux-uclibc/sysroot/usr/bin" test -z "/usr/lib/pkgconfig" || /bin/mkdir -p "/mnt/hee/new/buildroot-2012.08/output/host/usr/i586-unknown-linux-uclibc/sysroot/usr/lib/pkgconfig"
/usr/bin/install -c 'divine-config'
'/mnt/hee/new/buildroot-2012.08/output/host/usr/i586-unknown-linux-uclibc/sysroot/usr/bin/divine-config'
/usr/bin/install -c -m 644 'divine.pc'
'/mnt/hee/new/buildroot-2012.08/output/host/usr/i586-unknown-linux-uclibc/sysroot/usr/lib/pkgconfig/divine.pc' make[3]: Poistutaan hakemistosta
"/mnt/hee/new/buildroot-2012.08/output/build/divine-0.4.0"
make[2]: Poistutaan hakemistosta
"/mnt/hee/new/buildroot-2012.08/output/build/divine-0.4.0"
make[1]: Poistutaan hakemistosta
"/mnt/hee/new/buildroot-2012.08/output/build/divine-0.4.0"
for i in $(find /mnt/hee/new/buildroot-2012.08/output/host/usr/i586-unknown-linux-uclibc/sysroot/usr/lib* -name "*.la"); do cp -f $i $i~; /bin/sed -i -e "s:\(['= ]\)/usr:\\1/mnt/hee/new/buildroot-2012.08/output/host/usr/i586-unknown-linux-uclibc/sysroot/usr:g" $i; done
if test -n ; then \
echo "[7m>>> divine 0.4.0 "Fixing package configuration files"[27m" ;\
for file in ; do \
if [ -e
/mnt/hee/new/buildroot-2012.08/output/host/usr/i586-unknown-linux-uclibc/sysroot/usr/bin/${file}
]; then \
/bin/sed -i -e
"s,^prefix=.*,prefix=/mnt/hee/new/buildroot-2012.08/output/host/usr/i586-unknown-linux-uclibc/sysroot/usr,g"
\
-e"s,^exec_prefix=.*,exec_prefix=/mnt/hee/new/buildroot-2012.08/output/host/usr/i586-unknown-linux-uclibc/sysroot/usr,g" \
/mnt/hee/new/buildroot-2012.08/output/host/usr/i586-unknown-linux-uclibc/sysroot/usr/bin/${file}
;\
fi \
done \fi
[7m>>> divine 0.4.0 Fixing package configuration files[27m
[7m>>> divine 0.4.0 Installing to target[27m PATH="/mnt/hee/new/buildroot-2012.08/output/host/bin:/mnt/hee/new/buildroot-2012.08/output/host/usr/bin:/mnt/hee/new/buildroot-2012.08/output/host/usr/sbin/:/root/perl5/bin:/root/perl5/bin:/root/perl5/bin:/usr/lib/ccache:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin" PERLLIB="/mnt/hee/new/buildroot-2012.08/output/host/usr/lib/perl"/usr/bin/make -j4 DESTDIR=/mnt/hee/new/buildroot-2012.08/output/target
install -C /mnt/hee/new/buildroot-2012.08/output/build/divine-0.4.0/
make[1]: Siirryt??n hakemistoon
"/mnt/hee/new/buildroot-2012.08/output/build/divine-0.4.0"
Making install in include
make[2]: Siirryt??n hakemistoon
"/mnt/hee/new/buildroot-2012.08/output/build/divine-0.4.0/include"
make[3]: Siirryt??n hakemistoon
"/mnt/hee/new/buildroot-2012.08/output/build/divine-0.4.0/include"
9.1.2013 9:54, Thomas Petazzoni kirjoitti:
Quoted 10 lines
Dear Yann E. MORIN,
Quoted 2 lines · Yann E. MORIN
All in all, in my experience, that's roughly split in those four fourths. Comunity work like here, and professional work alike.
Agreed. In this specific case, I'd say about 5 lines of documentation are probably needed. I'm willing to provide the patch if needed.
Thomas
That would be great!
Regards
Stefan
Quoted 10 lines · Stefan Fr?berg
Hi Thomas
I tried that empty variable checking example you gave but it won't work. If I remove
DIVINE_CONFIG_FIXUPfrom divine.mk it will still spit the "Fixing package configuration files" messageThis is what it looks so far. Added your variable checking and removed redundant '\ from sed line as Arnout said.
if test -n $($(PKG)_CONFIG_FIXUP) ; then \
There should be quotes around "$($(PKG)_CONFIG_FIXUP)". test -n without
arguments is true, test -n "" is false.
Quoted 3 lines
$(call MESSAGE,"Fixing package configuration files") ;\
for file in $($(PKG)_CONFIG_FIXUP); do \
if [ -e $(STAGING_DIR)/usr/bin/$${file} ]; then \This condition should be removed, as Thomas said.
Regards,
Arnout
Quoted 8 lines
$(SED) "s,^prefix=.*,prefix=$(STAGING_DIR)/usr,g" \
-e
"s,^exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr,g" \
$(STAGING_DIR)/usr/bin/$${file} ;\
fi \
done \
fi
$(Q)touch $@[snip]
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
Hi Arnout
9.1.2013 22:31, Arnout Vandecappelle kirjoitti:
Quoted 7 lines · Stefan Fr?berg
Quoted 6 lines
Hi Thomas
I tried that empty variable checking example you gave but it won't work. If I remove
DIVINE_CONFIG_FIXUPfrom divine.mk it will still spit the "Fixing package configuration files" message
Quoted 8 lines
Quoted 4 lines
This is what it looks so far. Added your variable checking and removed redundant '\ from sed line as Arnout said.
if test -n $($(PKG)_CONFIG_FIXUP) ; then \There should be quotes around "$($(PKG)_CONFIG_FIXUP)". test -n
without arguments is true, test -n "" is false.
Ok
Quoted 10 lines
Quoted 3 lines
$(call MESSAGE,"Fixing package configuration files") ;\ for file in $($(PKG)_CONFIG_FIXUP); do \ if [ -e $(STAGING_DIR)/usr/bin/$${file} ]; then \This condition should be removed, as Thomas said.
Regards,
Arnout
Okay.
Here is the new version
$(Q)if test -n "$($(PKG)_CONFIG_FIXUP)" ; then \
$(call MESSAGE,"Fixing package configuration files") ;\
$(SED) "s,^prefix=.*,prefix=$(STAGING_DIR)/usr,g" \
-e "s,^exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr,g" \
$(addprefix$(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_FIXUP)) ;\
fi
If nothing else comes up then I will make a new patch of this sometime tomorrow
Regards
Stefan