-
Notifications
You must be signed in to change notification settings - Fork 196
setup a cleaner build environment #665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
setup a cleaner build environment #665
Conversation
Multiple build failures, relevant to this change. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=982974 |
C.UTF-8 does not exist in older distros, so I'm not sure if it is a good idea to set it unconditionally. |
https://unix.stackexchange.com/questions/597962/how-widespread-is-the-c-utf-8-locale suggests Arch Linux does not have it - so it's not about the age of the distro |
True, but this pull request just patches the build-recipe-dsc file... |
but it's about the distribution executing it, or do I get this wrong? |
The change only affects |
build-recipe-dsc
Outdated
@@ -95,7 +95,7 @@ dsc_build() { | |||
# this allows the build environment to be manipulated | |||
# and alternate build commands can be used | |||
DSC_BUILD_CMD="$(queryconfig --dist "$BUILD_DIST" --archpath "$BUILD_ARCH" --configdir "$CONFIG_DIR" substitute dsc:build_cmd)" | |||
test -z "$DSC_BUILD_CMD" && DSC_BUILD_CMD="dpkg-buildpackage -us -uc" | |||
test -z "$DSC_BUILD_CMD" && DSC_BUILD_CMD="LC_ALL=C.UTF-8 dpkg-buildpackage -us -uc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the right place?
Shouldn't we change build-pkg-deb
instead?
(but I am not sure who calls who to be honest)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably both need to be changed; also build-pkg-deb
needs to consistently run deb_chroot
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think this is OK now, right ?
e6ebce5
to
2a6beef
Compare
Any update on this? |
Multiple packages have run into build failures on our OBS. On closer inspection, it was concluded to be issues with the locale settings. These issues were never seen on the Debian buildd infrastructure because there the locale setting LC_ALL=C.UTF-8 is in use. This change keeps us closer to what is used in Debian buildds. openSUSE#665 Signed-off-by: Emanuele Aina <[email protected]>
7fbc66c
to
a68beef
Compare
Also added a couple of fixes we've been carrying in our setup for a while. |
@rickysarraf do you have any time to resolve this ? It would be nice to have this in OBS by default. |
For additional context on Debian buildd setting LC_ALL to C.UTF-8 see: https://lists.debian.org/debian-devel/2024/06/msg00188.html The lack of OBS also doing this also appears to be the cause of the recent breakage of installing fontconfig in OBS build environments reported here: https://lists.opensuse.org/archives/list/[email protected]/thread/C24P565DGJLSB4VDKHZVC7YK5KMQYUCP/ As such I would support the merging of this pull request. |
Multiple packages have run into build failures on our OBS. On closer inspection, it was concluded to be issues with the locale settings. These issues were never seen on the Debian buildd infrastructure because there the locale setting LC_ALL=C.UTF-8 is in use. This change keeps us closer to what is used in Debian buildds. openSUSE#665 Signed-off-by: Emanuele Aina <[email protected]>
To make the build environment a bit more reproducible, ensure the local timezone is set to UTC when building packages. Signed-off-by: Emanuele Aina <[email protected]>
…ilds Our OBS workers run under a `screen` session, which gives them an actual `$TERM` variable. Some packages like `color.js` then enable colored output, which confuses some other packages like `node-grunt-legacy-log` which use it in their testsuite and do not expect colored output. To avoid that, unset `TERM`. Signed-off-by: Emanuele Aina <[email protected]>
spymemcached package is FTBFS because a test expects a reverse lookup of 127.0.0.1 to return localhost, but current /etc/hosts configuration returns the hostname instead. Let's move this `hostname/ip` entry to the end of /etc/hosts so the reverse lookup of 127.0.0.1 returns localhost, which makes sense to be the default case usually. Signed-off-by: Ariel D'Alessandro <[email protected]>
a68beef
to
3a32943
Compare
Thanks @obbardc |
@rickysarraf, thanks! There are a few more related changes in https://gitlab.collabora.com/obs/obs-build/-/commits/collabora/main, could you please pick them up too? |
@andrewshadura Sure. I'm on it. Will check now and pick the ones relevant. |
I wonder if there is any scope to create some useful unit tests for building Debian packages (not as part of this pr of course, but if you're interested maybe we could open separate issue) ? |
…ence Signed-off-by: Andrej Shadura <[email protected]> merge fallout Signed-off-by: Ritesh Raj Sarraf <[email protected]>
Signed-off-by: Andrej Shadura <[email protected]>
… builds Since "su -" cleans the environment, we need to tell it to keep variables we want to preserve. To make this easier, create a new wrapper su_deb_chroot that runs chroot and su and sets up the environment. Since DEBIAN_* variables are only needed when preparing the build chroot, don’t set them in su_deb_chroot. Signed-off-by: Andrej Shadura <[email protected]>
Using su - will set the path to a default value, which for a normal user doesn't include /sbin.. However some debian package do rely on sbin being in the path it seems. So explictly set the path to match the default set by debuild, including sbin Signed-off-by: Sjoerd Simons <[email protected]>
@andrewshadura I have now pushed all the cherry-picked commits as well. Given that this branch was rebased against upstream's master, some code had diverged and I had to resolve conflicts during the cherry-pick. So, this PR could appreciate another round of quick review from you. |
I think it’s better to leave out 4e273c6. This was fixed in a different way upstream. |
4e273c6
to
01e5688
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
I will test this in more detail with the failing Debian packages later, to see if it fixes the problem.
Multiple packages have run into build failures on our OBS. On closer
inspection, it was concluded to be issues with the locale settings.
These issues were never seen on the Debian buildd infrastructure because
there the locale setting
LC_ALL=C.UTF-8
is in use.This change keeps us closer to what is used in Debian buildds.
Signed-off-by: Ritesh Raj Sarraf [email protected]