[U-Boot] [PATCH] tools: imx8image: return SUCCESS when the required files not found
When the required files to build a bootable imx8 image are not found, return EXIT_SUCCESS to avoid build CI system. And if the files are missing, give a error message during the build.
Signed-off-by: Peng Fan peng.fan@nxp.com --- tools/imx8image.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/imx8image.c b/tools/imx8image.c index e6b0a146b6..35409646f5 100644 --- a/tools/imx8image.c +++ b/tools/imx8image.c @@ -279,9 +279,9 @@ static void check_file(struct stat *sbuf, char *filename) int tmp_fd = open(filename, O_RDONLY | O_BINARY);
if (tmp_fd < 0) { - fprintf(stderr, "%s: Can't open: %s\n", + fprintf(stderr, "*** %s: Can't open: %s ***\n", filename, strerror(errno)); - exit(EXIT_FAILURE); + exit(EXIT_SUCCESS); }
if (fstat(tmp_fd, sbuf) < 0) { @@ -311,9 +311,9 @@ static void copy_file_aligned(int ifd, const char *datafile, int offset,
dfd = open(datafile, O_RDONLY | O_BINARY); if (dfd < 0) { - fprintf(stderr, "Can't open %s: %s\n", + fprintf(stderr, "*** Can't open %s: %s ***\n", datafile, strerror(errno)); - exit(EXIT_FAILURE); + exit(EXIT_SUCCESS); }
if (fstat(dfd, &sbuf) < 0) { @@ -365,9 +365,9 @@ static void copy_file (int ifd, const char *datafile, int pad, int offset)
dfd = open(datafile, O_RDONLY | O_BINARY); if (dfd < 0) { - fprintf(stderr, "Can't open %s: %s\n", + fprintf(stderr, "*** Can't open %s: %s ***\n", datafile, strerror(errno)); - exit(EXIT_FAILURE); + exit(EXIT_SUCCESS); }
if (fstat(dfd, &sbuf) < 0) { @@ -648,8 +648,8 @@ static int get_container_image_start_pos(image_t *image_stack, uint32_t align) if (img_sp->option == APPEND) { fd = fopen(img_sp->filename, "r"); if (!fd) { - fprintf(stderr, "Fail open first container file %s\n", img_sp->filename); - exit(EXIT_FAILURE); + fprintf(stderr, "*** Fail open first container file %s ***\n", img_sp->filename); + exit(EXIT_SUCCESS); }
ret = fread(&header, sizeof(header), 1, fd);
Hi Peng,
On 24/10/18 07:09, Peng Fan wrote:
When the required files to build a bootable imx8 image are not found, return EXIT_SUCCESS to avoid build CI system. And if the files are missing, give a error message during the build.
Thanks for this - anyway, I am thinking about if this is the correct solution. IMHO the current implementation in mkimage *is* already correct. Files are missing, mkimage cannot be completed with success, we do not get an image suitable for booting - an error is raised. What else ? There is nothing wrong on this :-).
However, we have to make CI happy - but mkimage could be called even outside a U-Boot build, and should be scriptable, that is it must report an error in case of failure. This is already provided.
Instead of changing mkimage, I thought to not call mkimage at all. We could add a test in arch/arm/mach-imx/Makefile to check if files for imx8 are available, and if not, we print warnings and Makefile returns with success. CI / Travis should be happy again without introducing a buggy behavior into mkimage. What do you think ?
Best regards, Stefano
Signed-off-by: Peng Fan peng.fan@nxp.com
tools/imx8image.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/imx8image.c b/tools/imx8image.c index e6b0a146b6..35409646f5 100644 --- a/tools/imx8image.c +++ b/tools/imx8image.c @@ -279,9 +279,9 @@ static void check_file(struct stat *sbuf, char *filename) int tmp_fd = open(filename, O_RDONLY | O_BINARY);
if (tmp_fd < 0) {
fprintf(stderr, "%s: Can't open: %s\n",
fprintf(stderr, "*** %s: Can't open: %s ***\n", filename, strerror(errno));
exit(EXIT_FAILURE);
exit(EXIT_SUCCESS);}
if (fstat(tmp_fd, sbuf) < 0) {
@@ -311,9 +311,9 @@ static void copy_file_aligned(int ifd, const char *datafile, int offset,
dfd = open(datafile, O_RDONLY | O_BINARY); if (dfd < 0) {
fprintf(stderr, "Can't open %s: %s\n",
fprintf(stderr, "*** Can't open %s: %s ***\n", datafile, strerror(errno));
exit(EXIT_FAILURE);
exit(EXIT_SUCCESS);}
if (fstat(dfd, &sbuf) < 0) {
@@ -365,9 +365,9 @@ static void copy_file (int ifd, const char *datafile, int pad, int offset)
dfd = open(datafile, O_RDONLY | O_BINARY); if (dfd < 0) {
fprintf(stderr, "Can't open %s: %s\n",
fprintf(stderr, "*** Can't open %s: %s ***\n", datafile, strerror(errno));
exit(EXIT_FAILURE);
exit(EXIT_SUCCESS);}
if (fstat(dfd, &sbuf) < 0) {
@@ -648,8 +648,8 @@ static int get_container_image_start_pos(image_t *image_stack, uint32_t align) if (img_sp->option == APPEND) { fd = fopen(img_sp->filename, "r"); if (!fd) {
fprintf(stderr, "Fail open first container file %s\n", img_sp->filename);exit(EXIT_FAILURE);
fprintf(stderr, "*** Fail open first container file %s ***\n", img_sp->filename);exit(EXIT_SUCCESS); } ret = fread(&header, sizeof(header), 1, fd);
Hi Stefano,
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: 2018年10月24日 15:31 To: Peng Fan peng.fan@nxp.com; sbabic@denx.de Cc: u-boot@lists.denx.de Subject: Re: [PATCH] tools: imx8image: return SUCCESS when the required files not found
Hi Peng,
On 24/10/18 07:09, Peng Fan wrote:
When the required files to build a bootable imx8 image are not found, return EXIT_SUCCESS to avoid build CI system. And if the files are missing, give a error message during the build.
Thanks for this - anyway, I am thinking about if this is the correct solution. IMHO the current implementation in mkimage *is* already correct. Files are missing, mkimage cannot be completed with success, we do not get an image suitable for booting - an error is raised. What else ? There is nothing wrong on this :-).
However, we have to make CI happy - but mkimage could be called even outside a U-Boot build, and should be scriptable, that is it must report an error in case of failure. This is already provided.
Instead of changing mkimage, I thought to not call mkimage at all. We could add a test in arch/arm/mach-imx/Makefile to check if files for imx8 are available, and if not, we print warnings and Makefile returns with success. CI / Travis should be happy again without introducing a buggy behavior into mkimage. What do you think ?
Just send out a new patch, please check whether it is ok. I also kicked a CI build, still running, https://travis-ci.org/MrVan/u-boot/builds
Thanks, Peng.
Best regards, Stefano
Signed-off-by: Peng Fan peng.fan@nxp.com
tools/imx8image.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/imx8image.c b/tools/imx8image.c index e6b0a146b6..35409646f5 100644 --- a/tools/imx8image.c +++ b/tools/imx8image.c @@ -279,9 +279,9 @@ static void check_file(struct stat *sbuf, char
*filename)
int tmp_fd = open(filename, O_RDONLY | O_BINARY);
if (tmp_fd < 0) {
fprintf(stderr, "%s: Can't open: %s\n",
fprintf(stderr, "*** %s: Can't open: %s ***\n", filename, strerror(errno));
exit(EXIT_FAILURE);
exit(EXIT_SUCCESS);}
if (fstat(tmp_fd, sbuf) < 0) {
@@ -311,9 +311,9 @@ static void copy_file_aligned(int ifd, const char *datafile, int offset,
dfd = open(datafile, O_RDONLY | O_BINARY); if (dfd < 0) {
fprintf(stderr, "Can't open %s: %s\n",
fprintf(stderr, "*** Can't open %s: %s ***\n", datafile, strerror(errno));
exit(EXIT_FAILURE);
exit(EXIT_SUCCESS);}
if (fstat(dfd, &sbuf) < 0) {
@@ -365,9 +365,9 @@ static void copy_file (int ifd, const char *datafile, int pad, int offset)
dfd = open(datafile, O_RDONLY | O_BINARY); if (dfd < 0) {
fprintf(stderr, "Can't open %s: %s\n",
fprintf(stderr, "*** Can't open %s: %s ***\n", datafile, strerror(errno));
exit(EXIT_FAILURE);
exit(EXIT_SUCCESS);}
if (fstat(dfd, &sbuf) < 0) {
@@ -648,8 +648,8 @@ static int get_container_image_start_pos(image_t
*image_stack, uint32_t align)
if (img_sp->option == APPEND) { fd = fopen(img_sp->filename, "r"); if (!fd) {
fprintf(stderr, "Fail open first container file %s\n",img_sp->filename);
exit(EXIT_FAILURE);
fprintf(stderr, "*** Fail open first container file %s ***\n",img_sp->filename);
exit(EXIT_SUCCESS); } ret = fread(&header, sizeof(header), 1, fd);--
===== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de ================================================================ =====
Hi Peng,
On 24/10/18 11:51, Peng Fan wrote:
Hi Stefano,
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: 2018年10月24日 15:31 To: Peng Fan peng.fan@nxp.com; sbabic@denx.de Cc: u-boot@lists.denx.de Subject: Re: [PATCH] tools: imx8image: return SUCCESS when the required files not found
Hi Peng,
On 24/10/18 07:09, Peng Fan wrote:
When the required files to build a bootable imx8 image are not found, return EXIT_SUCCESS to avoid build CI system. And if the files are missing, give a error message during the build.
Thanks for this - anyway, I am thinking about if this is the correct solution. IMHO the current implementation in mkimage *is* already correct. Files are missing, mkimage cannot be completed with success, we do not get an image suitable for booting - an error is raised. What else ? There is nothing wrong on this :-).
However, we have to make CI happy - but mkimage could be called even outside a U-Boot build, and should be scriptable, that is it must report an error in case of failure. This is already provided.
Instead of changing mkimage, I thought to not call mkimage at all. We could add a test in arch/arm/mach-imx/Makefile to check if files for imx8 are available, and if not, we print warnings and Makefile returns with success. CI / Travis should be happy again without introducing a buggy behavior into mkimage. What do you think ?
Just send out a new patch, please check whether it is ok.
Thanks - it is what I mind ;-)
I also kicked a CI build, still running, https://travis-ci.org/MrVan/u-boot/builds
Well, easy for me :-) I just wait your build is over, and then I merge and send a new PR to Tom !
Best regards, Stefano
Thanks, Peng.
Best regards, Stefano
Signed-off-by: Peng Fan peng.fan@nxp.com
tools/imx8image.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/imx8image.c b/tools/imx8image.c index e6b0a146b6..35409646f5 100644 --- a/tools/imx8image.c +++ b/tools/imx8image.c @@ -279,9 +279,9 @@ static void check_file(struct stat *sbuf, char
*filename)
int tmp_fd = open(filename, O_RDONLY | O_BINARY);
if (tmp_fd < 0) {
fprintf(stderr, "%s: Can't open: %s\n",
fprintf(stderr, "*** %s: Can't open: %s ***\n", filename, strerror(errno));
exit(EXIT_FAILURE);
exit(EXIT_SUCCESS);}
if (fstat(tmp_fd, sbuf) < 0) {
@@ -311,9 +311,9 @@ static void copy_file_aligned(int ifd, const char *datafile, int offset,
dfd = open(datafile, O_RDONLY | O_BINARY); if (dfd < 0) {
fprintf(stderr, "Can't open %s: %s\n",
fprintf(stderr, "*** Can't open %s: %s ***\n", datafile, strerror(errno));
exit(EXIT_FAILURE);
exit(EXIT_SUCCESS);}
if (fstat(dfd, &sbuf) < 0) {
@@ -365,9 +365,9 @@ static void copy_file (int ifd, const char *datafile, int pad, int offset)
dfd = open(datafile, O_RDONLY | O_BINARY); if (dfd < 0) {
fprintf(stderr, "Can't open %s: %s\n",
fprintf(stderr, "*** Can't open %s: %s ***\n", datafile, strerror(errno));
exit(EXIT_FAILURE);
exit(EXIT_SUCCESS);}
if (fstat(dfd, &sbuf) < 0) {
@@ -648,8 +648,8 @@ static int get_container_image_start_pos(image_t
*image_stack, uint32_t align)
if (img_sp->option == APPEND) { fd = fopen(img_sp->filename, "r"); if (!fd) {
fprintf(stderr, "Fail open first container file %s\n",img_sp->filename);
exit(EXIT_FAILURE);
fprintf(stderr, "*** Fail open first container file %s ***\n",img_sp->filename);
exit(EXIT_SUCCESS); } ret = fread(&header, sizeof(header), 1, fd);--
===== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de ================================================================ =====
Dear Peng Fan,
In message 20181024051500.12467-1-peng.fan@nxp.com you wrote:
When the required files to build a bootable imx8 image are not found, return EXIT_SUCCESS to avoid build CI system. And if the files are missing, give a error message during the build.
Full NAK.
Naked-by: Wolfgang Denk wd@denx.de
if (tmp_fd < 0) {
fprintf(stderr, "%s: Can't open: %s\n",
fprintf(stderr, "*** %s: Can't open: %s ***\n", filename, strerror(errno));
exit(EXIT_FAILURE);
exit(EXIT_SUCCESS);
An error like this must ALWAYS be reported by a proper return / exit code o fthe funtion / program, other wise it is impossible to use such tools in any automatic scripts, pipelines etc.
Anything else is just fundamentally broken. I am shocked to see such a patch.
Wolfgang Denk
participants (3)
-
Peng Fan -
Stefano Babic -
Wolfgang Denk