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.

Sign up for GitHub

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

Jump to bottom

Support for cross-platform RPM package generation #729

Open
wants to merge 1 commit into
base: 0.9.x
Choose a base branch
from
Open

Support for cross-platform RPM package generation #729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions pkg/make_rpm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,14 @@ class RpmBuilder(object):
RPMS_DIR = 'RPMS'
DIRS = [SOURCE_DIR, BUILD_DIR, RPMS_DIR, TEMP_DIR]

def __init__(self, name, version, release, arch, rpmbuild_path,
source_date_epoch=None,
def __init__(self, name, version, release, arch, target_arch,
rpmbuild_path, source_date_epoch=None,
debug=False):
self.name = name
self.version = helpers.GetFlagValue(version)
self.release = helpers.GetFlagValue(release)
self.arch = arch
self.target_arch = target_arch
self.files = []
self.rpmbuild_path = FindRpmbuild(rpmbuild_path)
self.rpm_path = None
Expand Down Expand Up @@ -354,6 +355,10 @@ def CallRpmBuild(self, dirname, rpmbuild_args):
'--buildroot=%s' % buildroot,
] # yapf: disable

# Target platform
if self.target_arch:
args += ['--target=%s' % self.target_arch]

# Macro-based RPM parameter substitution, if necessary inputs provided.
if self.preamble_file:
args += ['--define', 'build_rpm_options %s' % self.preamble_file]
Expand Down Expand Up @@ -462,7 +467,10 @@ def main(argv):
help='The release of the software being packaged.')
parser.add_argument(
'--arch',
help='The CPU architecture of the software being packaged.')
help='The CPU architecture of the machine on which it is built. '
'If the package is not architecture dependent, set this to "noarch".')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't this comment apply to targert_arch?

parser.add_argument('--target_arch',
help='The CPU architecture of the target platform the software being packaged for.')
parser.add_argument('--spec_file', required=True,
help='The file containing the RPM specification.')
parser.add_argument('--out_file', required=True,
Expand Down Expand Up @@ -501,7 +509,7 @@ def main(argv):
try:
builder = RpmBuilder(options.name,
options.version, options.release,
options.arch, options.rpmbuild,
options.arch, options.target_arch, options.rpmbuild,
source_date_epoch=options.source_date_epoch,
debug=options.debug)
builder.AddFiles(options.files)
Expand Down
26 changes: 19 additions & 7 deletions pkg/rpm_pfg.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def _make_absolute_if_not_already_or_is_macro(path):
# this can be inlined easily.
return path if path.startswith(("/", "%")) else "/" + path

#### Input processing helper functons.
#### Input processing helper functions.

# TODO(nacl, #459): These are redundant with functions and structures in
# pkg/private/pkg_files.bzl. We should really use the infrastructure provided
Expand Down Expand Up @@ -251,7 +251,7 @@ def _pkg_rpm_impl(ctx):
rpm_name,
ctx.attr.version,
ctx.attr.release,
ctx.attr.architecture,
ctx.attr.architecture if ctx.attr.architecture else ctx.attr.target_architecture,
)

outputs, output_file, output_name = setup_output_files(
Expand Down Expand Up @@ -432,6 +432,9 @@ def _pkg_rpm_impl(ctx):

args.append("--out_file=" + output_file.path)

if ctx.attr.target_architecture:
args.append("--target_arch=" + ctx.attr.target_architecture)

# Add data files.
if ctx.file.changelog:
files.append(ctx.file.changelog)
Expand Down Expand Up @@ -666,7 +669,7 @@ def _pkg_rpm_impl(ctx):
output_groups = {
"out": [default_file],
"rpm": [output_file],
"changes": changes
"changes": changes,
}
return [
OutputGroupInfo(**output_groups),
Expand Down Expand Up @@ -791,21 +794,30 @@ pkg_rpm = rule(
# funny if it's not provided. The contents of the RPM are believed to
# be set as expected, though.
"architecture": attr.string(
doc = """Package architecture.
doc = """Host architecture.

This currently sets the `BuildArch` tag, which influences the output
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's drop "currently" in these

architecture of the package.

Typically, `BuildArch` only needs to be set when the package is
Copy link
Collaborator

Choose a reason for hiding this comment

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

You might as well make the exact behavior and use clear and tie it back to Bazel definitions when the terms overlap. The points to make are

  • If the package is really architecture independent, say "noarch"
  • Otherwise specify a CPU architecture of the target platform your are building for.
  • If you say nothing, it will default to the CPU arch of your execution platform.

Note that when cross compiling the target and execution platforms may not match.

Copy link

Choose a reason for hiding this comment

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

@aiuto @ogalbxela when cross-compiling, target/exec may differ, but the logical default value would be the target.

When not cross-compiling, the target and exec match, so the logical default value would be the target.

Is there interest in setting the default to the target rather than exec platform? Or a risk of breaking change in the default ?

known to be cross-platform (e.g. written in an interpreted
language), or, less common, when it is known that the application is
only valid for specific architectures.
not architecture dependent (e.g. written in an interpreted
language).

When no attribute is provided, this will default to your host's
architecture. This is usually what you want.

""",
),
"target_architecture": attr.string(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need both? I have not figured out the value of setting both, since only one gets used.

doc = """Package architecture.

This currently sets the value for the "--target" argument to "rpmbuild"
to specify platform package is built for.

When no attribute is provided, this will default to your host's
architecture.
""",
),
"license": attr.string(
doc = """RPM "License" tag.

Expand Down
14 changes: 14 additions & 0 deletions tests/rpm/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,19 @@ pkg_rpm(
version = "1.1.1",
)

pkg_rpm(
name = "test_rpm_alien",
srcs = [
":test_pfg",
],
target_architecture = "alien",
description = """pkg_rpm test rpm for alien target platform description""",
license = "Apache 2.0",
release = "2222",
summary = "pkg_rpm test rpm for alien target platform summary",
version = "1.1.1",
)

# Just like the above one, except the compression is changed.
pkg_rpm(
name = "test_rpm-bzip2",
Expand Down Expand Up @@ -305,6 +318,7 @@ sh_library(
testonly = True,
srcs = [
":test_rpm",
":test_rpm_alien",
":test_rpm-bzip2",
":test_rpm_direct",
":test_rpm_manifest",
Expand Down
4 changes: 2 additions & 2 deletions tests/rpm/make_rpm_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def testSetupWorkdir(self):

with PrependPath([outer]):
# Create the builder and exercise it.
builder = make_rpm.RpmBuilder('test', '1.0', '0', 'x86', None)
builder = make_rpm.RpmBuilder('test', '1.0', '0', 'x86', 'x86', None)

# Create spec_file, test files.
WriteFile('test.spec', 'Name: test', 'Version: 0.1',
Expand Down Expand Up @@ -162,7 +162,7 @@ def testBuild(self):

with PrependPath([outer]):
# Create the builder and exercise it.
builder = make_rpm.RpmBuilder('test', '1.0', '0', 'x86', None)
builder = make_rpm.RpmBuilder('test', '1.0', '0', 'x86', 'x86', None)

# Create spec_file, test files.
WriteFile('test.spec', 'Name: test', 'Version: 0.1',
Expand Down
21 changes: 21 additions & 0 deletions tests/rpm/pkg_rpm_basic_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ def setUp(self):
self.runfiles = runfiles.Create()
self.test_rpm_path = self.runfiles.Rlocation(
"rules_pkg/tests/rpm/test_rpm.rpm")
self.test_rpm_alien_path = self.runfiles.Rlocation(
"rules_pkg/tests/rpm/test_rpm_alien.rpm")
self.test_rpm_direct_path = self.runfiles.Rlocation(
"rules_pkg/tests/rpm/test_rpm_direct.rpm")
self.test_rpm_bzip2_path = self.runfiles.Rlocation(
Expand Down Expand Up @@ -85,6 +87,25 @@ def test_basic_headers(self):
output, expected,
"RPM Tag {} does not match expected value".format(fieldname))

def test_basic_headers_alien_target(self):
fields = {
"NAME": b"test_rpm_alien",
"VERSION": b"1.1.1",
"RELEASE": b"2222",
"ARCH": b"alien",
"GROUP": b"Unspecified",
"SUMMARY": b"pkg_rpm test rpm for alien target platform summary",
}
for fieldname, expected in fields.items():
output = subprocess.check_output([
"rpm", "-qp", "--queryformat", "%{" + fieldname + "}",
self.test_rpm_alien_path
])

self.assertEqual(
output, expected,
"RPM Tag {} does not match expected value".format(fieldname))

def test_contents(self):
manifest_file = self.runfiles.Rlocation(
"rules_pkg/tests/rpm/manifest.csv")
Expand Down