We previously whitelisted this return code because split packages can
frequently conflict each other, so makepkg -i is *expected* to fail in
such a case. However, there is no good reason to let this succeed if the
pkgbase only builds one pkgname -- that will always be a severe issue.
Add a check for how many split
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
The -U option was initially introduced in commit
cda9cf436b in order to enable running
makechrootpkg as root, delegating to another, manually selected, user to
perform various non-root tasks (given that makepkg was modified to throw
fatal errors when run as root without the option of --asroot to disable
that). However, it was only ever implemented for the --verifysource
option outside of the chroot, and the builduser inside the chroot is
created with the same uid as the makechrootpkg invoker. It needs to run
as the same uid, because it needs rw access to $startdir and $SRCDEST!
Additionally this lets the invoking user more easily inspect the build
directory in case of problems...
The correct solution for this is to properly implement the initial
intention of the -U option, and make it override the autodetection of
the "invoking user" which is normally done by inspecting $SUDO_USER.
This is then used as the single source of truth for "who am I pretending
to be".
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
Packages should never be getting downgraded... unless a package is
pulled from testing, e.g. for example if gcc9 totally breaks the linux
kernel. In such cases, the master repo says there is a downgrade, so
we'd better go with that. Basically, ensure that packages match the repo
they are being built against. Consistency at all costs!
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
noconfirm is wrong here, as we don't want to accept the default answer
-- we want to install the new package, even if it conflicts and provides
an existing one. After all, we explicitly asked for it.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
And pass them on to download_sources outside the chroot.
Fixes FS#35652
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
In commit bd826752c9, support for short
options was added to the heuristic for --noextract, but in the process,
we changed to loop over the set of user options plus the builtin
defaults for inside the chroot. This was wrong, as we only care about
the user options -- moreover, it prevents us from adding verifysource
support *outside* the chroot, for options that are also chroot options,
like --holdver.
Also remove uselessly duplicated line.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
When parsing paths to automatically make available to the container, the
":" is used internally by systemd-nspawn to signify destinations in the
container. Replace automatically with "\:" for the mounts that we set
up, in order to safely handle a working directory etc. that contains
this character.
For bind options exposed to the user, it is assumed the user takes care
of passing systemd-nspawn compatible paths themselves.
Fixes FS#60845
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
Using the literal strings "true" and "false" is inaccurate and may
result in uncertainty of whether it is set when doing string comparison,
or simply rely on the shell implementation of treating the string as a
command builtin, then executing the value as a shell command. Emulate
makepkg, which makes heavy use of shell arithmetic for this purpose.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
This reverts (the bulk of) commit 2fd5931a8c.
Reducing globals makes little sense in in a oneshot bash script, but
reduces code clarity and in fact resulted in bugs because even the
commit author couldn't keep track of the script state.
An exit was changed to a return, even though that made no sense outside
of a function, and has been duly returned to being an exit. This was
never tested and later papered over by wrapping the entire script in a
main() function and then calling the function for hysterical raisins.
The functiony nature of sync_chroot/delete_chroot is preserved, as those
functions demonstrate meaningfully standalone functionality -- who
knows? we may want to reuse this. Everything else is tightly bound to
the internal logic of makechrootpkg.
Completely separate functionality that was silently implemented in the
original commit is also preserved:
- declare a couple of variables as locals
- move the abort-on-no-PKGBUILD outside the install_packages function
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
This reverts commit 49088b0860.
The fundamental intention was flawed and broken, it caused annoying
issues and regressions, and the self-avowed sole purpose of the change
was so that a downstream project could *post-modify the script and
source it as a library*.
That is not okay. You don't wrap non-factorable code in a function
called main() and call it a library. The only possible use for this is
to treat makechrootpkg *internals* as a library, which is not supported.
Downstream projects that wish to use the functionality of makechrootpkg
should treat makepkg as a command with a public API in the form of
command line options. That is kind of how commands of all kinds work,
since forever. That is how all users of makechrootpkg *except for
parabola* use it.
Arguments that "it saves us the cost of fork+exec to bash" are simply
invalid.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
Since makepkg.conf is a bash-compatible configuration file, it must be
sourced.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
This reverts commit 6d1992909c.
It has never worked. In commit c86823a2d4
it was noted that it compared the device numbers for [[ $1 = $1 ]] which
was a useless check and always returned true, for *any* btrfs
filesystem. Now that the function is corrected to compare [[ $1 = $2 ]]
the check is still useless, but this time because it always returns
false -- btrfs subvolumes on the same filesystem do *not* share device
numbers.
So let's go back to the original working implementation that only
matters in terms of makechrootpkg, and just checks if makechrootpkg's
root working directory is btrfs (in which case we know it will be a
subvolume because mkarchroot will create it that way).
This restores our special support for the btrfs filesystem.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
If makechrootpkg is called as non-root, the {SRC,SRCPKG,PKG,LOG}DEST,
MAKEFLAGS and PACKAGER environment variables are lost in the call to
check_root().
Add these to the passed keepenv list so that they are preserved instead.
makechrootpkg's download_sources() leaves a stray directory if
"makepkg --verifysource" failed. We use "setup_workdir" instead
of "mktemp -d", because this ensures the correct garbage collection.
Signed-off-by: Erich Eckner <git@eckner.net>
makepkg 5.1 implements error codes, and 14 means that installing the
packages after they were built has failed. We don't care about this
error and would like makechrootpkg to succeed regardless, e.g. for split
packages that are mutually exclusive.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
chown support "$user:$group" but also "$user:" which infers $group
rather than leaving it as root. This looks up the group name in cases
where the default group is e.g. "users" and users do not get their own
unique groups.
In pacman-git commit d8717a6a9666ec80c8645d190d6f9c7ab73084ac makepkg
started checking that the setuid/setgid bit could be removed on the
$BUILDDIR in order to prevent this propagating to the packages
themselves. Unfortunately, this requires the temporary builddir used
during the --verifysource stage of makepkg, to be owned by $makepkg_user
which was not the case as it is created as root using mktemp (and given
world rwx in addition to the restricted deletion bit.)
Obviously makepkg cannot chmod a directory that it does not own. Fix
this by making $makepkg_user the owner of that directory, as should have
been the case all along.
(Giving world rwx is illogical on general principle. The fact that this
is a workaround for makepkg demanding these directories be writable even
when they are not going to be used for the makepkg options in question,
is not justification for being careless.)
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Previously, makechrootpkg hardcoded ~/.gnupg. Therefore, if a user
uses a custom GPG home directory, the siganture checking would fail.
Now makechrootpkg uses $GNUPGHOME, with a fallback to ~/.gnupg.
Signed-off-by: Emiel Wiedijk <me@aimileus.nl>
Don't use error-prone logic e.g.
foo=true; if $foo ...
This completely fails to act as expected when the variable is unset
because of unrelated bugs.
While this merely causes the default behavior to be "false" rather than
"true" in such cases, it is better to fail to enable explicitly
requested behavior (which will be noticed by the user) than to simply
upgrade to this behavior for free (which may not seem to have any
obvious cause).
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Fixes regression in 2fd5931a8c
$run_namcap will always be set to ""
`if $not_a_var; then ...; fi` is always truthful when $not_a_var is
unset or equal to "" and the `then` clause will always be run.
I'm not sure why global state variables need to be cloned locally for
their sole explicit purpose.
But for now this patch implements the minimum necessary work to properly
pass the "do I want namcap" variable into prepare_chroot() according to
the current logic flow.
Note that I have still not thorougly tested makechrootpkg.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Recent development versions of makepkg support reproducible builds
through the environment variable SOURCE_DATE_EPOCH. Pass this variable
through makechrootpkg to makepkg when available.
Also initialize SOURCE_DATE_EPOCH whenever running archbuild to enforce
reproducible builds for repository packages.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Levente Polyak <anthraxx@archlinux.org>
A couple of the comments noting which globals are used by functions are
outdated/wrong.
- download_sources() : Remove USER from the list. It was always wrong.
Originally, it should have been SUDO_USER (not USER), but I should have
removed it entirely in 4f23609.
- move_products() : Add SRCPKGDEST to the list. Though the commit adding
the comment was only recently upstreamed (as 2fd5931), it originated in
2013 in a commit that has since been rebased many times. Anyway, in
this rebasing, it missed move_products() starting to pay attention to
SRCPKGDEST in fd1be1b (since nothing made git think there was a
"conflict").
The reason it wasn't moved before was just to keep the diffs
(with --ignore-all-space) smaller, to make merging and rebasing work
easier. Moving code around in a file tends to make that difficult.
But, readability wise, it belongs in main().
nspawn does not give us a controlling terminal, hence we ignore
interrupts. Apparently this was lost in systemd at some point.
Hack around this by reopening the console to make it the controlling
terminal.
Coredumps from build chroots are not generally useful. Prevent
them from being generated.
Avoids a lot of annoyance from the GCC testsuite spawning lots of
systemd-coredump processes.
Just set the soft limit so the user can still raise it in the PKGBUILD
if they insist.
Commit 58968cf fixed symlinks for package products in $startdir in
light of the simplified chroot setup. However, a similar change needs
to be made for source-package products. This was an easy omission to
make because makechrootpkg does not produce source-pakcages by
default.
The bug isn't currently triggered, but I accidentally did trigger when I
was trying to modify the command a bit. I figure a "caution" sign would be
helpful to any future developers.
What this is really doing is fixing a conflict that I had incorrectly
resolved when rebasing what became 2fd5931 onto cda9cf4. Of course,
because of dynamic scoping, everything worked out, and everything worked as
intended.
Before cda9cf4, it was appropriate for download_sources to take src_owner
as an argument, but after cda9cf4, it is now appropriate to take
makepkg_user as an argument. However, it still takes src_owner as an
argument, but pays 0 attention to it; instead looking at makepkg_user which
it happily inherited because of dynamic scoping.
So change it to take makepkg_user as the argument.
This is inspired by the thought that went in to the delete_chroot
is_subvolume commit.
sync_chroot($chrootdir, $copydir) copies `$chrootdir/root` to `$copydir`.
That seems a little silly; why do we care about "$chrootdir"? Have it just
be sync_chroot(source, destination) like every other sync/copy command.
Where this becomes tricky is check to decide if we are going to use btrfs
subvolumes or not. We don't care if "$source/.." is on btrfs; the root
could be a directly-mounted subvolume, but and the destination could be
another subvolume of the same btrfs mounted somewhere else.
The things we do care about are:
- The source is a btrfs subvolume (so that we can snapshot it)
- The source is on the same filesystem as the directory that the copy will
be created in.
- If the destination exists:
* that it is not a mountpoint (so that we can delete and recreate it)
* that it is a btrfs subvolume (so that we can quickly delete it)
On the last point, it isn't necessary for creating the new snapshot, just
for quick deletion. That can be a separate check, where we use regular
`rm` for deleting the existing copy, but use subvolume snapshots for
creating the new one.
Also, shorten the "Synchronizing" message to only include the full path
to the copy if it was specified.
The capslocked variable names in the Usage comment were references to
things in Parabola's tools, that didn't make much sense here out of
context.
First of all, it ran `is_btrfs "$chrootdir"` to decide if it was on
btrfs, but $chrootdir wasn't defined locally; it just happens to work
because $chrootdir was defined in main(). (I noticed this because in
Parabola, it is called differently, so $chrootdir was empty).
So I was tempted to just change it to `is_btrfs "$copydir"`, but if
$copydir is just a regular directory on a btrfs filesystem, then it
It would leave much of $copydir intact. What we really care about is
if $copydir is a btrfs subvolume; which we can check by combining the
is_btrfs check with inspecting the inum of the directory.
I put this combined check in lib/archroot.sh:is_subvolume.
https://lists.archlinux.org/pipermail/arch-projects/2013-September/003901.html
This means wrapping variable initialization in init_variables(), and the
main program routine in main().
I did NOT put `shopt -s nullglob` in to a function.
It make make sense to move init_variables() down into the main()
function, instead of having it as a separate function up top (if this
done, then the `-g` flag passed to `declare` in init_variables() can
be dropped). However, in interest of keeping the `diff -w` small, and
merges/rebases simpler, this isn't done here.
In sync_chroot(), this makes the messages be a bit more precise with
exactly which thing they are syncing where. This is based on my users
expressing confusion at what is going on (especially when something is
taking a long time, and they have to blame something for blocking).
With these changes, I haven't gotten such confusion in a long time
(but maybe my users just got used to it).
In delete_chroot(), this changes "temporary copy" to "chroot copy",
since in Parabola's version of the tools, the function can get called
from other places, and it isn't necessarily operating on a temporary
copy.
Even though main() doesn't call `set -u`; this way the functions will
continue to work if copied into an environment with `set -u`, or so
that we are ready if we ever want to start using `set -u`.
Rather than them simply being named blocks of code with braces around
them.
That is: have them take things via arguments rather than global
variables.
Specific notes:
- create_chroot->sync_chroot:
I pulled out locking the destination chroot; getting that lock is
now the caller's responsibility. It still handles locking the
source chroot though.
I pulled the `if [[ ! -d $copydir ]] || $clean_first;` check out; it is
now the caller's responsibility to use that check when deciding if to
call sync_chroot.
However, when pulling that check out, I left it as `if true;`, to
keep an indentation level. This patch has had to be rebased/merged
many times, and changing the indentation is a sure way to make that
go less smoothly; I'm not going to re-indent this block until I see
the check removed in the git.archlinux.org/devtools.git repository.
- install_packages:
1. Receive the list of packages as arguments, rather than a global
variable.
2. Make the caller responsible for looking at PKGBUILD. From the
name and arguments, one would never expect it to look at PKGBUILD.
Eli Schwartz
Eli Schwartz via arch-projects
Matt Robinson
Erich Eckner
Evangelos Foutras
Emiel Wiedijk
Bartłomiej Piotrowski
Luke Shumaker
Jan Alexander Steffens (heftig)
Martchus