Skip to content
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

Introduce an rpm-controlled per-build directory #2885

Merged
merged 2 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 29 additions & 11 deletions build/build.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const char *name,
const char *sb, int test, StringBuf * sb_stdoutp)
{
char *scriptName = NULL;
char * buildDir = rpmGenPath(spec->rootDir, "%{_builddir}", "");
char * buildDir = rpmGenPath(spec->rootDir, "%{builddir}", "");
char * buildSubdir = rpmGetPath("%{?buildsubdir}", NULL);
char * buildCmd = NULL;
char * buildTemplate = NULL;
Expand All @@ -126,6 +126,10 @@ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const char *name,
rpmRC rc = RPMRC_FAIL; /* assume failure */

switch (what) {
case RPMBUILD_MKBUILDDIR:
mTemplate = "%{__spec_builddir_template}";
mPost = "%{__spec_builddir_post}";
mCmd = "%{__spec_builddir_cmd}";
case RPMBUILD_PREP:
mTemplate = "%{__spec_prep_template}";
mPost = "%{__spec_prep_post}";
Expand Down Expand Up @@ -195,18 +199,11 @@ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const char *name,

(void) fputs(buildTemplate, fp);

if (what != RPMBUILD_PREP && what != RPMBUILD_RMBUILD && buildSubdir[0] != '\0')
if (what != RPMBUILD_MKBUILDDIR && what != RPMBUILD_PREP && what != RPMBUILD_RMBUILD && buildSubdir[0] != '\0')
fprintf(fp, "cd '%s'\n", buildSubdir);

if (what == RPMBUILD_RMBUILD) {
if (rpmMacroIsDefined(spec->macros, "specpartsdir")) {
char * buf = rpmExpand("%{specpartsdir}", NULL);
fprintf(fp, "rm -rf '%s'\n", buf);
free(buf);
}
if (buildSubdir[0] != '\0')
fprintf(fp, "rm -rf '%s' '%s.gemspec'\n",
buildSubdir, buildSubdir);
fprintf(fp, "rm -rf '%s'\n", spec->buildDir);
} else if (sb != NULL)
fprintf(fp, "%s", sb);

Expand Down Expand Up @@ -308,6 +305,24 @@ static rpmRC doCheckBuildRequires(rpmts ts, rpmSpec spec, int test)
return rc;
}

static rpmRC doBuildDir(rpmSpec spec, int test, StringBuf *sbp)
{
char *doDir = rstrscat(NULL,
"rm -rf '", spec->buildDir, "'\n",
"mkdir -p '", spec->buildDir, "'\n",
NULL);

rpmRC rc = doScript(spec, RPMBUILD_MKBUILDDIR, "%mkbuilddir",
doDir, test, sbp);
if (rc) {
rpmlog(RPMLOG_ERR,
_("failed to create package build directory %s: %s\n"),
spec->buildDir, strerror(errno));
}
free(doDir);
return rc;
}

static rpmRC buildSpec(rpmts ts, BTA_t buildArgs, rpmSpec spec, int what)
{
rpmRC rc = RPMRC_OK;
Expand Down Expand Up @@ -377,13 +392,16 @@ static rpmRC buildSpec(rpmts ts, BTA_t buildArgs, rpmSpec spec, int what)
if (!rpmSpecGetSection(spec, RPMBUILD_BUILDREQUIRES) && sourceOnly) {
/* don't run prep if not needed for source build */
/* with(out) dynamic build requires*/
what &= ~(RPMBUILD_PREP);
what &= ~(RPMBUILD_PREP|RPMBUILD_MKBUILDDIR);
}

if ((what & RPMBUILD_CHECKBUILDREQUIRES) &&
(rc = doCheckBuildRequires(ts, spec, test)))
goto exit;

if ((what & RPMBUILD_MKBUILDDIR) && (rc = doBuildDir(spec, test, sbp)))
goto exit;

if ((what & RPMBUILD_PREP) &&
(rc = doScript(spec, RPMBUILD_PREP, "%prep",
rpmSpecGetSection(spec, RPMBUILD_PREP),
Expand Down
6 changes: 3 additions & 3 deletions build/files.c
Original file line number Diff line number Diff line change
Expand Up @@ -1655,7 +1655,7 @@ static rpmRC processMetadataFile(Package pkg, FileList fl,
fn = rpmGenPath(fl->buildRoot, NULL, fileName);
absolute = 1;
} else
fn = rpmGenPath("%{_builddir}", "%{?buildsubdir}", fileName);
fn = rpmGenPath("%{builddir}", "%{?buildsubdir}", fileName);

switch (tag) {
default:
Expand Down Expand Up @@ -2245,7 +2245,7 @@ int readManifest(rpmSpec spec, const char *path, const char *descr, int flags,
if (*path == '/') {
fn = rpmGetPath(path, NULL);
} else {
fn = rpmGenPath("%{_builddir}", "%{?buildsubdir}", path);
fn = rpmGenPath("%{builddir}", "%{?buildsubdir}", path);
}
fd = fopen(fn, "r");

Expand Down Expand Up @@ -2389,7 +2389,7 @@ static void processSpecialDir(rpmSpec spec, Package pkg, FileList fl,
char *mkdocdir = rpmExpand("%{__mkdir_p} $", sdenv, NULL);
StringBuf docScript = newStringBuf();
int count = sd->entriesCount;
char *basepath = rpmGenPath(spec->rootDir, "%{_builddir}", "%{?buildsubdir}");
char *basepath = rpmGenPath(spec->rootDir, "%{builddir}", "%{?buildsubdir}");
ARGV_t *files = xmalloc(sizeof(*files) * count);
int i, j;

Expand Down
44 changes: 27 additions & 17 deletions build/parsePreamble.c
Original file line number Diff line number Diff line change
Expand Up @@ -1267,28 +1267,38 @@ int parsePreamble(rpmSpec spec, int initialPackage)
}
}

/*
* Expand buildroot one more time to get %{version} and the like
* from the main package, validate sanity. The spec->buildRoot could
* still contain unexpanded macros but it cannot be empty or '/', and it
* can't be messed with by anything spec does beyond this point.
*/
if (initialPackage) {
if (checkForRequiredForBuild(pkg->header)) {
goto exit;
}

char *buildRoot = rpmGetPath(spec->buildRoot, NULL);
free(spec->buildRoot);
spec->buildRoot = buildRoot;
rpmPushMacro(spec->macros, "buildroot", NULL, spec->buildRoot, RMIL_SPEC);
if (*buildRoot == '\0') {
rpmlog(RPMLOG_ERR, _("%%{buildroot} couldn't be empty\n"));
goto exit;
}
if (rstreq(buildRoot, "/")) {
rpmlog(RPMLOG_ERR, _("%%{buildroot} can not be \"/\"\n"));
goto exit;
if (!spec->buildDir) {
/* Grab top builddir on first entry as we'll override _builddir */
if (!rpmMacroIsDefined(spec->macros, "_top_builddir")) {
char *top_builddir = rpmExpand("%{_builddir}", NULL);
rpmPushMacroFlags(spec->macros, "_top_builddir", NULL,
top_builddir, RMIL_GLOBAL, RPMMACRO_LITERAL);
free(top_builddir);
}

/* Using release here causes a buildid no-recompute test to fail */
spec->buildDir = rpmExpand("%{_top_builddir}/%{NAME}-%{VERSION}-%{_arch}", NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please not include architecture? That can leak into builds and cause problems. That's why I had to make an adjustment in redhat-rpm-config for %_vpath_builddir: https://src.fedoraproject.org/rpms/redhat-rpm-config/c/e0cfcc0fc76a7642faabb25c5e348d6a1314ace2

Copy link
Member Author

@pmatilai pmatilai Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that's the same thing, this is one level below it. It gets a bit hysterical if we can't name our directories the way we want just because Doxygen.

I admit this is tricky stuff, like debuginfo expectations breaking if NVRA is used there. But that's at least actually related to rpm somehow.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and FWIW, I would love to get the full build paths somehow abstracted out anyhow. I don't know how much is really doable in that space, but on systems that support this kind of stuff, builds would run temp directory inside their build directory and writes limited to their own build directory, bind-mounted so that the build thinks its running in /build directory instead of /home/whatever/somewhere path etc.

/* Override toplevel _builddir for backwards compatibility */
rpmPushMacroFlags(spec->macros, "_builddir", NULL, spec->buildDir,
RMIL_SPEC, RPMMACRO_LITERAL);

/* A user-oriented, unambiguous name for the thing */
rpmPushMacroFlags(spec->macros, "builddir", NULL, spec->buildDir,
RMIL_SPEC, RPMMACRO_LITERAL);

spec->buildRoot = rpmGetPath(spec->buildDir, "/BUILDROOT", NULL);
rpmPushMacroFlags(spec->macros, "buildroot", NULL, spec->buildRoot,
RMIL_SPEC, RPMMACRO_LITERAL);

char *specparts = rpmGetPath(spec->buildDir, "/SPECPARTS", NULL);
rpmPushMacroFlags(spec->macros, "specpartsdir", NULL, specparts,
RMIL_SPEC, RPMMACRO_LITERAL);
free(specparts);
pmatilai marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
15 changes: 5 additions & 10 deletions build/parseSpec.c
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,7 @@ static rpmRC parseBuildsystem(rpmSpec spec)
}

static rpmSpec parseSpec(const char *specFile, rpmSpecFlags flags,
const char *buildRoot, int recursing);
int recursing);

static rpmRC parseSpecSection(rpmSpec *specptr, int secondary)
{
Expand Down Expand Up @@ -1124,7 +1124,7 @@ static rpmRC parseSpecSection(rpmSpec *specptr, int secondary)
if (!rpmMachineScore(RPM_MACHTABLE_BUILDARCH, spec->BANames[x]))
continue;
rpmPushMacro(NULL, "_target_cpu", NULL, spec->BANames[x], RMIL_RPMRC);
spec->BASpecs[index] = parseSpec(spec->specFile, spec->flags, spec->buildRoot, 1);
spec->BASpecs[index] = parseSpec(spec->specFile, spec->flags, 1);
if (spec->BASpecs[index] == NULL) {
spec->BACount = index;
goto errxit;
Expand Down Expand Up @@ -1190,7 +1190,7 @@ static rpmRC parseSpecSection(rpmSpec *specptr, int secondary)


static rpmSpec parseSpec(const char *specFile, rpmSpecFlags flags,
const char *buildRoot, int recursing)
int recursing)
{
rpmSpec spec;

Expand All @@ -1199,12 +1199,7 @@ static rpmSpec parseSpec(const char *specFile, rpmSpecFlags flags,

spec->specFile = rpmGetPath(specFile, NULL);
pushOFI(spec, spec->specFile);
/* If buildRoot not specified, use default %{buildroot} */
if (buildRoot) {
spec->buildRoot = xstrdup(buildRoot);
} else {
spec->buildRoot = rpmGetPath("%{?buildroot:%{buildroot}}", NULL);
}

rpmPushMacro(NULL, "_docdir", NULL, "%{_defaultdocdir}", RMIL_SPEC);
rpmPushMacro(NULL, "_licensedir", NULL, "%{_defaultlicensedir}", RMIL_SPEC);
spec->recursing = recursing;
Expand Down Expand Up @@ -1292,7 +1287,7 @@ static rpmRC finalizeSpec(rpmSpec spec)
rpmSpec rpmSpecParse(const char *specFile, rpmSpecFlags flags,
const char *buildRoot)
{
rpmSpec spec = parseSpec(specFile, flags, buildRoot, 0);
rpmSpec spec = parseSpec(specFile, flags, 0);
if (spec && !(flags & RPMSPEC_NOFINALIZE)) {
finalizeSpec(spec);
}
Expand Down
2 changes: 1 addition & 1 deletion build/policies.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ static ModuleRec newModule(const char *path, const char *name,
ModuleRec mod;
uint8_t *raw = NULL;
ssize_t rawlen = 0;
const char *buildDir = "%{_builddir}/%{?buildsubdir}/";
const char *buildDir = "%{builddir}/%{?buildsubdir}/";

if (!path) {
rpmlog(RPMLOG_ERR, _("%%semodule requires a file path\n"));
Expand Down
1 change: 1 addition & 0 deletions build/rpmbuild_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ struct rpmSpec_s {

char * specFile; /*!< Name of the spec file. */
char * buildRoot;
char * buildDir;
const char * rootDir;

struct OpenFileInfo * fileStack;
Expand Down
4 changes: 3 additions & 1 deletion build/spec.c
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ rpmSpec newSpec(void)
spec->sourcePackage = NULL;

spec->buildRoot = NULL;
spec->buildDir = NULL;

spec->buildRestrictions = headerNew();
spec->BANames = NULL;
Expand All @@ -260,6 +261,7 @@ rpmSpec rpmSpecFree(rpmSpec spec)
freeStringBuf(spec->parsed);

spec->buildRoot = _free(spec->buildRoot);
spec->buildDir = _free(spec->buildDir);
spec->specFile = _free(spec->specFile);

closeSpec(spec);
Expand Down Expand Up @@ -298,7 +300,7 @@ rpmSpec rpmSpecFree(rpmSpec spec)
spec->pool = rpmstrPoolFree(spec->pool);

spec->buildHost = _free(spec->buildHost);

spec = _free(spec);

return spec;
Expand Down
1 change: 1 addition & 0 deletions docs/manual/buildprocess.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ title: rpm.org - Package Build Process
* Parse the [spec](spec.md)
* If buildarch detected parse spec multiple times - once for each arch with `_target_cpu` macro set
* Build will iterate over all the spec variants and build multiple versions
* Execute internal `%mkbuilddir` script to create `%builddir`
* Check static build requires
* Execute present [build scriptlets](spec.md#build-scriptlets)
* `%prep`
Expand Down
5 changes: 2 additions & 3 deletions docs/manual/dynamic_specs.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ build results.

The files need to be placed in the **%{specpartsdir}** (also available
as **$RPM_SPECPARTS_DIR** in the build scripts) and have a
**.specpart** postfix. The directory is created by **%setup**. Default
location is **%{_builddir}/%{buildsubdir}-SPECPARTS** which is beside
the **%{buildsubdir}**. Scripts must not create it themselves but must
**.specpart** postfix. The directory is created by **%setup**.
Scripts must not create it themselves but must
either fail if it is not present or switch to an alternative that does
not require the feature. They should give an error message that
dynamic spec generation is not supported on the given RPM version when
Expand Down
3 changes: 2 additions & 1 deletion include/rpm/rpmbuild.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ enum rpmBuildFlags_e {
RPMBUILD_BUILDREQUIRES = (1 << 20), /*!< Execute %%buildrequires. */
RPMBUILD_DUMPBUILDREQUIRES = (1 << 21), /*!< Write buildrequires.nosrc.rpm. */
RPMBUILD_CONF = (1 << 22), /*!< Execute %%conf. */
RPMBUILD_MKBUILDDIR = (1 << 23), /*!< Internal use only */

RPMBUILD_NOBUILD = (1 << 31) /*!< Don't execute or package. */
};
Expand Down Expand Up @@ -77,7 +78,7 @@ typedef struct rpmBuildArguments_s * BTA_t;
*
* @param specFile path to spec file
* @param flags flags to control operation
* @param buildRoot buildRoot override or NULL for default
* @param buildRoot unused
* @return new spec control structure
*/
rpmSpec rpmSpecParse(const char *specFile, rpmSpecFlags flags,
Expand Down
25 changes: 12 additions & 13 deletions macros.in
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@
%{?_find_debuginfo_dwz_opts} \\\
%{?_find_debuginfo_opts} \\\
%{?_debugsource_packages:-S debugsourcefiles.list} \\\
"%{_builddir}/%{?buildsubdir}"\
"%{builddir}/%{?buildsubdir}"\
%{nil}

# Template for debug information sub-package.
Expand Down Expand Up @@ -266,15 +266,6 @@ Supplements: (%{name} = %{version}-%{release} and langpacks-%{1})\
# The directory where newly built source packages will be written.
%_srcrpmdir %{_topdir}/SRPMS

# The directory where buildroots will be created.
%_buildrootdir %{_topdir}/BUILDROOT

# Build root path, where %install installs the package during build.
%buildroot %{_buildrootdir}/%{NAME}-%{VERSION}-%{RELEASE}.%{_arch}

# Path for spec file snippets generated during build
%specpartsdir %{_builddir}/%{buildsubdir}-SPECPARTS

# Directory where temporaray files can be created.
%_tmppath %{_var}/tmp

Expand All @@ -286,7 +277,7 @@ Supplements: (%{name} = %{version}-%{release} and langpacks-%{1})\
# Macros that are initialized as a side effect of rpmrc and/or spec
# file parsing.
#
# The sub-directory (relative to %{_builddir}) where sources are compiled.
# The sub-directory (relative to %{builddir}) where sources are compiled.
# This macro is set after processing %setup, either explicitly from the
# value given to -n or the default name-version.
#
Expand Down Expand Up @@ -763,7 +754,7 @@ Supplements: (%{name} = %{version}-%{release} and langpacks-%{1})\
%___build_cmd %{?_sudo:%{_sudo} }%{?_remsh:%{_remsh} %{_remhost} }%{?_remsudo:%{_remsudo} }%{?_remchroot:%{_remchroot} %{_remroot} }%{___build_shell} %{___build_args}
%___build_pre_env \
RPM_SOURCE_DIR=\"%{_sourcedir}\"\
RPM_BUILD_DIR=\"%{_builddir}\"\
RPM_BUILD_DIR=\"%{builddir}\"\
RPM_OPT_FLAGS=\"%{optflags}\"\
RPM_LD_FLAGS=\"%{?build_ldflags}\"\
RPM_ARCH=\"%{_arch}\"\
Expand Down Expand Up @@ -793,7 +784,7 @@ Supplements: (%{name} = %{version}-%{release} and langpacks-%{1})\
%{___build_pre_env} \
%[%{verbose}?"set -x":""]\
umask 022\
cd \"%{_builddir}\"\
cd \"%{builddir}\"\


#%___build_body %{nil}
Expand All @@ -815,6 +806,14 @@ Supplements: (%{name} = %{version}-%{release} and langpacks-%{1})\
# ---- Scriptlet templates.
# Macro(s) that expand to a command and script that is executed.
#
%__spec_builddir_shell %{nil}
%__spec_builddir_args %{nil}
%__spec_builddir_cmd %{nil}
%__spec_builddir_pre %{nil}
%__spec_builddir_body %{%nil}
%__spec_builddir_post %{nil}
%__spec_builddir_template %{nil}

%__spec_prep_shell %{___build_shell}
%__spec_prep_args %{___build_args}
%__spec_prep_cmd %{___build_cmd}
Expand Down
18 changes: 14 additions & 4 deletions tests/data/SPECS/simple.spec
Original file line number Diff line number Diff line change
@@ -1,22 +1,32 @@
%bcond setup 0

Name: simple
Version: 1.0
Release: 1
Summary: Simple test package
Group: Testing
License: GPL
BuildArch: noarch
Source: source-noroot.tar.gz

%description
%{summary}

%install
mkdir -p $RPM_BUILD_ROOT/opt/bin
cat << EOF > $RPM_BUILD_ROOT/opt/bin/simple
%if %{with setup}
%prep
%setup -C
%endif

%build
cat << EOF > simple
#!/bin/sh
echo yay
EOF
chmod a+x simple

chmod a+x $RPM_BUILD_ROOT/opt/bin/simple
%install
mkdir -p $RPM_BUILD_ROOT/opt/bin
cp simple $RPM_BUILD_ROOT/opt/bin/

%post
touch /var/lib/simple
Expand Down