Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
On Wednesday, June 15, 2011 17:37:33 Ilya Yanok wrote: > Sometimes we want to build common tools without configuring for specific > target. Currently top Makefile has some support for this but it doesn't > work. This patch tries to fix this. i think you'll need to split this up in logical sep changesets since you are fixing different problems. > Things changed: > - config.mk disable 'ld script not found error' in case if we are > building tools only. i dont like copying & pasting the same logic in multiple places. this is how code rots and bug fixes diverge. considering LDSCRIPT/CONFIG_SYS_LDSCRIPT only get used by the top level u-boot file, and only when the system is configured, i wonder if we shouldnt just rip it out of config.mk and into the top level Makefile. let's see what Scott thinks ... > - Makefilemkimage relies on autogenerated version so we need to > move $(VERSION_FILE) rule out of ifeq and make tools rule depend on it. the VERSION_FILE changes look fine > - tools/Makefile put common/env_embedded.o and envcrc.o to object list > conditionally. This fixes errors during dependency generation. pretty sure this breaks board builds. if the only thing this fixes is a harmless warning when generating dependency files, then i say ignore it. after all, this is how it has always worked in the past and no one really cared. -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
Dear Mike Frysinger, In message <201106181503.10550.vap...@gentoo.org> you wrote: > > pretty sure this breaks board builds. if the only thing this fixes is a > harmless warning when generating dependency files, then i say ignore it. > after all, this is how it has always worked in the past and no one really > cared. There have been no warnings when generating dependencies. Changes that cause new warnings need to be fixed. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de What was sliced bread the greatest thing since? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
On Saturday, June 18, 2011 16:11:17 Wolfgang Denk wrote: > Mike Frysinger wrote: > > pretty sure this breaks board builds. if the only thing this fixes is a > > harmless warning when generating dependency files, then i say ignore it. > > after all, this is how it has always worked in the past and no one really > > cared. > > There have been no warnings when generating dependencies. yes, there has. you probably dont run `make tools` (at least, in an unconfigured tree), so you've never noticed. > Changes that cause new warnings need to be fixed. you'll have to define "new" here. the warning he refers to has been there for many many releases, and for pretty much as long as i can remember. i dont mind fixing warnings, but i do mind breaking the building of boards to fix a harmless warning. -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
On Sunday, June 19, 2011 13:55:13 Ilya Yanok wrote: > On 18.06.2011 23:03, Mike Frysinger wrote: > >> - tools/Makefile put common/env_embedded.o and envcrc.o to object list > >> > >> conditionally. This fixes errors during dependency generation. > > > > pretty sure this breaks board builds. if the only thing this fixes is a > > I'm sorry but I can't see how this can break the builds. Could you > please be more specific? I've tried to build some boards, it actually > works... i might be thinking of a different env_embedded situation. a different problem with your patch to tools/Makefile: you copied the same logic multiple times which means more bitrot. why dont you do something like: diff --git a/tools/Makefile b/tools/Makefile index 623f908..97f83f8 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -48,17 +48,21 @@ CONFIG_NETCONSOLE = y CONFIG_SHA1_CHECK_UB_IMG = y endif +# Merge all the different vars for envcrc into one +ENVCRC-$(CONFIG_ENV_IS_EMBEDDED) = y +ENVCRC-$(CONFIG_ENV_IS_IN_DATAFLASH) = y +ENVCRC-$(CONFIG_ENV_IS_IN_EEPROM) = y +ENVCRC-$(CONFIG_ENV_IS_IN_FLASH) = y +ENVCRC-$(CONFIG_ENV_IS_IN_ONENAND) = y +ENVCRC-$(CONFIG_ENV_IS_IN_NAND) = y +ENVCRC-$(CONFIG_ENV_IS_IN_NVRAM) = y +ENVCRC-$(CONFIG_ENV_IS_IN_SPI_FLASH) = y +CONFIG_BUILD_ENVCRC ?= $(ENVCRC-y) + # Generated executable files BIN_FILES-$(CONFIG_LCD_LOGO) += bmp_logo$(SFX) BIN_FILES-$(CONFIG_VIDEO_LOGO) += bmp_logo$(SFX) -BIN_FILES-$(CONFIG_ENV_IS_EMBEDDED) += envcrc$(SFX) -BIN_FILES-$(CONFIG_ENV_IS_IN_DATAFLASH) += envcrc$(SFX) -BIN_FILES-$(CONFIG_ENV_IS_IN_EEPROM) += envcrc$(SFX) -BIN_FILES-$(CONFIG_ENV_IS_IN_FLASH) += envcrc$(SFX) -BIN_FILES-$(CONFIG_ENV_IS_IN_ONENAND) += envcrc$(SFX) -BIN_FILES-$(CONFIG_ENV_IS_IN_NAND) += envcrc$(SFX) -BIN_FILES-$(CONFIG_ENV_IS_IN_NVRAM) += envcrc$(SFX) -BIN_FILES-$(CONFIG_ENV_IS_IN_SPI_FLASH) += envcrc$(SFX) +BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX) BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX) BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX) BIN_FILES-$(CONFIG_INCA_IP) += inca-swap-bytes$(SFX) @@ -67,7 +71,7 @@ BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX) # Source files which exist outside the tools directory -EXT_OBJ_FILES-y += common/env_embedded.o +EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o EXT_OBJ_FILES-y += common/image.o EXT_OBJ_FILES-y += lib/crc32.o EXT_OBJ_FILES-y += lib/md5.o @@ -77,7 +81,7 @@ EXT_OBJ_FILES-y += lib/sha1.o OBJ_FILES-$(CONFIG_LCD_LOGO) += bmp_logo.o OBJ_FILES-$(CONFIG_VIDEO_LOGO) += bmp_logo.o NOPED_OBJ_FILES-y += default_image.o -OBJ_FILES-y += envcrc.o +OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc.o NOPED_OBJ_FILES-y += fit_image.o OBJ_FILES-$(CONFIG_CMD_NET) += gen_eth_addr.o OBJ_FILES-$(CONFIG_CMD_LOADS) += img2srec.o > > harmless warning when generating dependency files, then i say ignore it. > > after all, this is how it has always worked in the past and no one really > > cared. > > Yep, they are harmless but they are not warnings but rather scary errors > actually. ;) I think it's better to fix them. i guess my threshold for being scared is a bit higher :p -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
Hi Mike, thanks for your comments. On 18.06.2011 23:03, Mike Frysinger wrote: > i think you'll need to split this up in logical sep changesets since you are > fixing different problems. Ok, will do. >> Things changed: >> - config.mk disable 'ld script not found error' in case if we are >> building tools only. > > i dont like copying & pasting the same logic in multiple places. this is how > code rots and bug fixes diverge. Agreed. How should I fix this? > considering LDSCRIPT/CONFIG_SYS_LDSCRIPT only get used by the top level > u-boot > file, and only when the system is configured, i wonder if we shouldnt just > rip > it out of config.mk and into the top level Makefile. > > let's see what Scott thinks ... I see. Let's wait for Scott's comments then. >> - tools/Makefile put common/env_embedded.o and envcrc.o to object list >> conditionally. This fixes errors during dependency generation. > > pretty sure this breaks board builds. if the only thing this fixes is a I'm sorry but I can't see how this can break the builds. Could you please be more specific? I've tried to build some boards, it actually works... > harmless warning when generating dependency files, then i say ignore it. > after all, this is how it has always worked in the past and no one really > cared. Yep, they are harmless but they are not warnings but rather scary errors actually. ;) I think it's better to fix them. Regards, Ilya. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
Hi Mike, On 19.06.2011 22:27, Mike Frysinger wrote: > i might be thinking of a different env_embedded situation. a different > problem with your patch to tools/Makefile: you copied the same logic multiple > times which means more bitrot. Yes, that's a good point. Your patch looks good to me. Should I write a commit message and repost it or will you? Regards, Ilya. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
On Sunday, June 19, 2011 18:46:42 Ilya Yanok wrote: > On 19.06.2011 22:27, Mike Frysinger wrote: > > i might be thinking of a different env_embedded situation. a different > > problem with your patch to tools/Makefile: you copied the same logic > > multiple times which means more bitrot. > > Yes, that's a good point. Your patch looks good to me. Should I write a > commit message and repost it or will you? respin your patches taking my suggestions into account -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
On Sat, 18 Jun 2011 15:03:09 -0400 Mike Frysinger wrote: > considering LDSCRIPT/CONFIG_SYS_LDSCRIPT only get used by the top level > u-boot > file, and only when the system is configured, i wonder if we shouldnt just > rip > it out of config.mk and into the top level Makefile. > > let's see what Scott thinks ... Sounds fine. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
Dear Ilya, In message <1308173853-20178-1-git-send-email-ya...@emcraft.com> you wrote: > Sometimes we want to build common tools without configuring for specific > target. Currently top Makefile has some support for this but it doesn't > work. This patch tries to fix this. > Things changed: > - config.mk disable 'ld script not found error' in case if we are > building tools only. > - Makefilemkimage relies on autogenerated version so we need to > move $(VERSION_FILE) rule out of ifeq and make tools rule depend on it. > - tools/Makefile put common/env_embedded.o and envcrc.o to object list > conditionally. This fixes errors during dependency generation. > > Signed-off-by: Ilya Yanok > --- > Makefile | 34 +- > config.mk |2 ++ > tools/Makefile | 19 --- > 3 files changed, 35 insertions(+), 20 deletions(-) Is it correct to assume that this commit has been obsolteted by the following commits: 249b53a 2011-10-06 20:19:42 +0200 Build timestamp_autogenerated.h without config a76406f 2011-10-06 20:20:15 +0200 Safer timestamp_autogenerated.h generation 9f87658 2011-10-06 20:21:16 +0200 ublimage: NAND block size isn't set at build-time ? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Every living thing wants to survive. -- Spock, "The Ultimate Computer", stardate 4731.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
Dear Wolfgang, On 17.10.2011 23:42, Wolfgang Denk wrote: >> Sometimes we want to build common tools without configuring for specific >> target. Currently top Makefile has some support for this but it doesn't >> work. This patch tries to fix this. >> Things changed: >> - config.mk disable 'ld script not found error' in case if we are >> building tools only. >> - Makefilemkimage relies on autogenerated version so we need to >> move $(VERSION_FILE) rule out of ifeq and make tools rule depend on it. >> - tools/Makefile put common/env_embedded.o and envcrc.o to object list >> conditionally. This fixes errors during dependency generation. >> >> Signed-off-by: Ilya Yanok >> --- >> Makefile | 34 +- >> config.mk |2 ++ >> tools/Makefile | 19 --- >> 3 files changed, 35 insertions(+), 20 deletions(-) > > Is it correct to assume that this commit has been obsolteted by > the following commits: > > 249b53a 2011-10-06 20:19:42 +0200 Build timestamp_autogenerated.h without > config > a76406f 2011-10-06 20:20:15 +0200 Safer timestamp_autogenerated.h > generation > 9f87658 2011-10-06 20:21:16 +0200 ublimage: NAND block size isn't set at > build-time Not exactly. I've reposted my patch splitting it into three as Mike suggested, and all these patches are already applied. The commits you mention deal with the related problems that were introduced later during the development. Regards, Ilya. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
Dear Ilya Yanok, In message <4e9c8ef9.3030...@emcraft.com> you wrote: > > > Is it correct to assume that this commit has been obsolteted by > > the following commits: ... > Not exactly. I've reposted my patch splitting it into three as Mike > suggested, and all these patches are already applied. > > The commits you mention deal with the related problems that were > introduced later during the development. So it should still be applied? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de I am more bored than you could ever possibly be. Go back to work. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools: make it possible to build tools unconfigured
Dear Wolfgang, On 18.10.2011 00:35, Wolfgang Denk wrote: >>> Is it correct to assume that this commit has been obsolteted by >>> the following commits: > ... >> Not exactly. I've reposted my patch splitting it into three as Mike >> suggested, and all these patches are already applied. >> >> The commits you mention deal with the related problems that were >> introduced later during the development. > > So it should still be applied? No, you can just drop it. It's already applied long ago (well, its second version). Regards, Ilya. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot