swupdate binaries contain absolute paths, leading to unreproducible builds

[Gerard Salvatella]

Several binaries, such as `swupdate`, `swupdate.debug` or `libswupdate`, contain absolute paths (e.g. `/path/to/git/core/swupdate.c` or `/path/to/git/core/handler.c`). This leads to unreproducible builds, specially in build systems like yocto/bitbake, where warnings of the following type show up: `File /usr/bin/swupdate in package swupdate contains reference to TMPDIR`.

These paths should be made relative, which is most likely solvable from the `Makefiles`. Could someone with more expertise in the project point me to where I should be looking for (Makefile variables, etc)? I'd be happy to issue some patches.

Thanks in advance.

[Stefano Babic]

Hallo Gerard,

On 17.06.24 11:02, Gerard Salvatella wrote:
> Several binaries, such as `swupdate`, `swupdate.debug` or `libswupdate`,
> contain absolute paths (e.g. `/path/to/git/core/swupdate.c` or
> `/path/to/git/core/handler.c`).

Is this still an issue ? I fixed this last year with:

commit e4ff96a314953273c9573d01307a8cf68199c184
Author: Stefano Babic <sba...@denx.de>
Date: Wed Apr 5 15:40:45 2023 +0200

Add __FILE_NAME__ macro to remove full path in log

So the stripped version (target) shouldn't have any full path. Full path
are still present in the unstripped version.

Which version is affected ? If I run current version, I do not see the
full path, just the relative one in the logs.

> This leads to unreproducible builds,
> specially in build systems like yocto/bitbake, where warnings of the
> following type show up: `File /usr/bin/swupdate in package swupdate
> contains reference to TMPDIR`.

This is something else - and yes, SWUpdate is using the TMPDIR variable.
And it has always used, so changing raises a compatibility issue.
bitbake checks if the binary contains reference to host TMPDIR, that is
/tmp, but this is also the reference on the target and it is used.

Anyway, this has nothing to do with reproducible build, it is another QA
issue, and SWUpdate is *using* TMPDIR. I guess that the QA is away if
SWUpdate points to /run/..., but if the directory was not created,
SWUpdate does not start. So it is a compatibility issue.

>
> These paths should be made relative, which is most likely solvable from
> the `Makefiles`.

Makefile are using CURDIR to get the list of source files, and CURDIR is
the absolute path set by make for the working directory.

> Could someone with more expertise in the project point
> me to where I should be looking for (Makefile variables, etc)? I'd be
> happy to issue some patches.
>

Best regards,
Stefano Babic

> Thanks in advance.
>
> --
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/swupdate/e14ede86-4f43-4107-b7f9-2cef4535de1en%40googlegroups.com <https://groups.google.com/d/msgid/swupdate/e14ede86-4f43-4107-b7f9-2cef4535de1en%40googlegroups.com?utm_medium=email&utm_source=footer>.

[Gerard Salvatella]

> Is this still an issue ? I fixed this last year with:

No, this doesn't seem to be an issue here.

> This is something else - and yes, SWUpdate is using the TMPDIR variable.

This seems like the root cause then. By defining `const char* TMPDIR` the contents of $TMPDIR are stored in the data/text segment. For yocto, this is an absolute path to the working directory, hence the warning. Would it then make sense to suppress this particular warning with a SKIP_INSANE = "buildpaths" in the recipe?

[Stefano Babic]

Hi,

On 18.06.24 09:36, Gerard Salvatella wrote:
>
> > Is this still an issue ? I fixed this last year with:
>
> No, this doesn't seem to be an issue here.
>
> > This is something else - and yes, SWUpdate is using the TMPDIR variable.
>
> This seems like the root cause then. By defining `const char* TMPDIR`
> the contents of $TMPDIR are stored in the data/text segment. For yocto,
> this is an absolute path to the working directory, hence the warning.

No, checking into meta, I do not think this is the issue. bitbake
evaluates TMPDIR on host with getenv("TMPDIR"), and this is compared. So
the lines in SWUpdate raising this should be just:

if (env_tmpdir == NULL) {
TMPDIR = (char*)"/tmp/";
return TMPDIR;
}

Because default is /tmp if not set.

> Would it then make sense to suppress this particular warning with a
> SKIP_INSANE = "buildpaths" in the recipe?

It is just to drop the warning - in future it makes sense to move it
away from /tmp.

Best regards,
Stefano Babic

> https://groups.google.com/d/msgid/swupdate/e14ede86-4f43-4107-b7f9-2cef4535de1en%40googlegroups.com <https://groups.google.com/d/msgid/swupdate/e14ede86-4f43-4107-b7f9-2cef4535de1en%40googlegroups.com> <https://groups.google.com/d/msgid/swupdate/e14ede86-4f43-4107-b7f9-2cef4535de1en%40googlegroups.com?utm_medium=email&utm_source=footer <https://groups.google.com/d/msgid/swupdate/e14ede86-4f43-4107-b7f9-2cef4535de1en%40googlegroups.com?utm_medium=email&utm_source=footer>>.

>
> --
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/swupdate/da498646-043a-460f-a321-5a15ca40ed8fn%40googlegroups.com <https://groups.google.com/d/msgid/swupdate/da498646-043a-460f-a321-5a15ca40ed8fn%40googlegroups.com?utm_medium=email&utm_source=footer>.

[Gerard Salvatella]

> No, checking into meta, I do not think this is the issue. bitbake

> evaluates TMPDIR on host with getenv("TMPDIR"), and this is compared

When using `strings` with the binaries in e.g. $WORKDIR/package/usr/bin/swupdate 

I get references to source files, e.g. $WORKDIR/git/core/swupdate.c. If I understood correctly,

this comes from the fact that `const char* TMPDIR = get_tmpdir()` retrieves its value from the

environment at compile time. In this case this seems to be the problem then, since `/tmp` is not

used at all.

> It  is just to drop the warning - in future it makes sense to move it
> away from /tmp.

So should I just send a patch with the warning disabled for the time being?