lua: fix install of lua.pc
PATCH 1/1 lua: fix install of lua.pc
install works even when the directory pkgconfig is not already created
see http://autobuild.buildroot.net/results/101/101c89e1d6aee942a0b1c4e4f3daf8ac2414a56c/
--- package/lua/lua.mk | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
package/lua/lua.mk +12 -4
@@ -65,6 +65,14 @@ endif HOST_LUA_CFLAGS = -Wall -fPIC -DLUA_USE_DLOPEN -DLUA_USE_POSIX HOST_LUA_MYLIBS = -ldl +define LUA_CONFIGURE_CMDS + $(SED) "s/@MYLIBS@/$(LUA_MYLIBS)/" $(@D)/etc/lua.pc +endef + +define HOST_LUA_CONFIGURE_CMDS + $(SED) "s/@MYLIBS@/$(HOST_LUA_MYLIBS)/" $(@D)/etc/lua.pc +endef + define LUA_BUILD_CMDS $(TARGET_MAKE_ENV) $(MAKE) \ CC="$(TARGET_CC)" RANLIB="$(TARGET_RANLIB)" \
@@ -86,8 +94,8 @@ endef define LUA_INSTALL_STAGING_CMDS $(TARGET_MAKE_ENV) $(MAKE) INSTALL_TOP="$(STAGING_DIR)/usr" -C $(@D) install - sed -e "s/@MYLIBS@/$(LUA_MYLIBS)/g" $(@D)/etc/lua.pc \ - > $(STAGING_DIR)/usr/lib/pkgconfig/lua.pc + $(INSTALL) -m 0644 -D $(@D)/etc/lua.pc \ + $(STAGING_DIR)/usr/lib/pkgconfig/lua.pc endef define LUA_INSTALL_TARGET_CMDS
@@ -96,8 +104,8 @@ endef define HOST_LUA_INSTALL_CMDS $(HOST_MAKE_ENV) $(MAKE) INSTALL_TOP="$(HOST_DIR)" -C $(@D) install - sed -e "s/@MYLIBS@/$(HOST_LUA_MYLIBS)/g" $(@D)/etc/lua.pc \ - > $(HOST_DIR)/lib/pkgconfig/lua.pc + $(INSTALL) -m 0644 -D $(@D)/etc/lua.pc \ + $(HOST_DIR)/lib/pkgconfig/lua.pc endef $(eval $(generic-package))
Signature · 1 line
2.11.0
PATCH 1/1 lua: fix install of lua.pc
Hello,
Quoted 5 lines · Francois Perrad
installworks even when the directory pkgconfig is not already createdsee http://autobuild.buildroot.net/results/101/101c89e1d6aee942a0b1c4e4f3daf8ac2414a56c/
Signed-off-by Francois Perrad <[email protected]>
This commit log doesn't explain why you're moving things to configure commands. And in fact, I believe the original code in which we do the SED at install time was more logical. So I've kept that, and just added the missing "mkdir -p" first.
See:
https://git.buildroot.org/buildroot/commit/?id=25a2650086ad9fe0e22e2a2c3d4e64cae396fcf1
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 lua: fix install of lua.pc
2017-08-14 22:25 GMT+02:00 Thomas Petazzoni <
[email protected]>:
Quoted 10 lines
Hello,
Quoted 3 lines · Francois Perrad
installworks even when the directory pkgconfig is not already created101c89e1d6aee942a0b1c4e4f3daf8ac2414a56c/
Quoted 2 lines
Signed-off-by Francois Perrad <[email protected]>
Quoted 10 lines
This commit log doesn't explain why you're moving things to configure commands. And in fact, I believe the original code in which we do the SED at install time was more logical. So I've kept that, and just added the missing "mkdir -p" first.
See:
https://git.buildroot.org/buildroot/commit/?id=25a2650086ad9fe0e22e2a2c3d4e64cae396fcf1
Thomas
The issue that we try to fix, was introduced 5 days ago, by the commit [lua: fix pkg-config file](
https://git.busybox.net/buildroot/commit/package/lua?id=8d845683e37640d33c186c0091ccce6ae3ef0777
)
My patch restore the previous install commands.
If the lua.pc must be modified with configuration data, the logical step for this is the configure step.
Your patch is the little one which fixes the issue, but it doesn't see the root cause.
Fran?ois
Quoted 12 lines
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170815/458be893/attachment.html>
PATCH 1/1 lua: fix install of lua.pc
Hello,
Quoted 11 lines · Fran?ois Perrad
The issue that we try to fix, was introduced 5 days ago, by the commit [lua: fix pkg-config file](
https://git.busybox.net/buildroot/commit/package/lua?id=8d845683e37640d33c186c0091ccce6ae3ef0777
)My patch restore the previous install commands.
If the lua.pc must be modified with configuration data, the logical step for this is the configure step.Your patch is the little one which fixes the issue, but it doesn't see the root cause.
Well, whether SED'ing those .pc files should be part of the configure step or the install step can really be discussed. If those .pc files were used for the build process, then indeed, generating them should be part of the configure step. But as far as I know, they are not used during the build step. Those .pc files are only used for other packages who want to link against lua. Therefore, generating them/installing them in the install step is quite OK I believe, but I agree it's a matter of definition of what each step should do, so there might be different views/opinions.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 lua: fix install of lua.pc
2017-08-15 10:39 GMT+02:00 Thomas Petazzoni <
[email protected]>:
Quoted 18 lines
Hello,
Quoted 3 lines · Fran?ois Perrad
The issue that we try to fix, was introduced 5 days ago, by the commit [lua: fix pkg-config file](
https://git.busybox.net/buildroot/commit/package/lua?id=
8d845683e37640d33c186c0091ccce6ae3ef0777Quoted 7 lines
)
My patch restore the previous install commands.
If the lua.pc must be modified with configuration data, the logical step for this is the configure step.Your patch is the little one which fixes the issue, but it doesn't see
the
Quoted 1 line
root cause.
Quoted 11 lines
Well, whether SED'ing those .pc files should be part of the configure step or the install step can really be discussed. If those .pc files were used for the build process, then indeed, generating them should be part of the configure step. But as far as I know, they are not used during the build step. Those .pc files are only used for other packages who want to link against lua. Therefore, generating them/installing them in the install step is quite OK I believe, but I agree it's a matter of definition of what each step should do, so there might be different views/opinions.
In order to keep a code base clean , I am against mixing the responsability of different steps.
In my point of view, the install step must only:
- copying files
- modifying the mode of files
- rename files
- create symbolic links
Fran?ois
Quoted 8 lines
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170815/83b29822/attachment.html>
PATCH 1/1 lua: fix install of lua.pc
Quoted 7 lines · Fran?ois Perrad
In order to keep a code base clean , I am against mixing the responsability of different steps.
In my point of view, the install step must only:
- copying files
- modifying the mode of files
- rename files
- create symbolic links
The current situation, at least, is also that fixing up files is done in
(post-)install. Cfr. $(PKG)_CONFIG_SCRIPTS), fixup of libtool files. And several packages already do additional fixups in post-install.
In this particular case, however, it does make more sense to make this
modification in the configure step. Well, except that the .pc file is something we add ourselves with a patch... So in fact, in this particular case, it would make more sense to do it in post-install, but to get the .pc file from package/lua/lua.pc.in instead of patching lua to get it. Except of course in case the patch has been accepted upstream, then it would make more sense to do what you propose.
So, in my opinion, there are two possibilities:
1. 0004-lua-pc.patch has been accepted upstream. In that case, the proper thing to do would be to:
a. patch lua's Makefile to also substitute @MYLIBS@
b. send the resulting patch upstream
2. lua.pc has not been accepted upstream. In that case, the proper thing to do would be to replace the patches with package/lua/lua.pc.in
In either case, however, this is more a matter of making things nicer, it
doesn't fix anything and it hardly makes things more maintainable. Only if lua.pc is upstream, then Joerg's fix should be improved and also sent upstream.
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: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 1/1 lua: fix install of lua.pc
2017-08-15 12:32 GMT+02:00 Arnout Vandecappelle <[email protected]>:
Quoted 42 lines · Fran?ois Perrad
Quoted 1 line
In order to keep a code base clean , I am against mixing the
responsability of
Quoted 6 lines
different steps.
In my point of view, the install step must only:
- copying files
- modifying the mode of files
- rename files
- create symbolic linksThe current situation, at least, is also that fixing up files is done in
(post-)install. Cfr. $(PKG)_CONFIG_SCRIPTS), fixup of libtool files. And several
packages already do additional fixups in post-install.In this particular case, however, it does make more sense to make this
modification in the configure step. Well, except that the .pc file is something
we add ourselves with a patch... So in fact, in this particular case, it would
make more sense to do it in post-install, but to get the .pc file from package/lua/lua.pc.in instead of patching lua to get it. Except of course in
case the patch has been accepted upstream, then it would make more sense to do
what you propose.So, in my opinion, there are two possibilities:
1. 0004-lua-pc.patch has been accepted upstream. In that case, the proper thing
to do would be to:
a. patch lua'sMakefileto also substitute @MYLIBS@
b. send the resulting patch upstream2. lua.pc has not been accepted upstream. In that case, the proper thing to do
would be to replace the patches with package/lua/lua.pc.in
Quoted 7 lines
In either case, however, this is more a matter of making things nicer, it
doesn't fix anything and it hardly makes things more maintainable. Only if lua.pc is upstream, then Joerg's fix should be improved and also sent upstream.
Patches will be not upstreamed, below the story:
- In Lua 5.1.x serie, a lua.pc was included in the upstream distribution. But it was a static file which was not updated with the configuration used during the build.
- With the Lua 5.2.x serie, the upstream team decides to remove it. I create a patch in serie 5.2 (and after in 5.3) which restores the same behavior like in 5.1.
This allows to install a static lua.pc in the 3 series.
These days, J?rg Krause makes lua.pc dynamic with correct data (ie. @MYLIBS@) in serie 5.2 & 5.3.
A common template lua.pc.in could be written with only 3 variables :LUA_VERSION, LUA_SERIE, MYLIB.
Fran?ois
Quoted 4 lines
Regards,
Arnout
Quoted 8 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: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170815/a5f8badc/attachment.html>
PATCH 1/1 lua: fix install of lua.pc
In reply to Thomas PetazzoniQuoted 10 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas Petazzoni <[email protected]> writes:
Hello,
Quoted 5 lines · Francois Perrad
installworks even when the directory pkgconfig is not already createdsee http://autobuild.buildroot.net/results/101/101c89e1d6aee942a0b1c4e4f3daf8ac2414a56c/
Signed-off-by Francois Perrad <[email protected]>
Quoted 7 lines
This commit log doesn't explain why you're moving things to configure commands. And in fact, I believe the original code in which we do the SED at install time was more logical. So I've kept that, and just added the missing "mkdir -p" first.
See:
https://git.buildroot.org/buildroot/commit/?id=25a2650086ad9fe0e22e2a2c3d4e64cae396fcf1
Committed to 2017.02.x (after adding back /usr to HOST_DIR), thanks.
Signature · 1 line
Bye, Peter Korsgaard