PR#143: fedora: use rpmautospec by default - fedora-rust/rust2rpm

fedora-rust / rust2rpm

#143 fedora: use rpmautospec by default

Merged 2 years ago by ngompa. Opened 2 years ago by zbyszek.

fedora-rust/ zbyszek/rust2rpm fedora-rpmautospec-by-default into master

fedora: use rpmautospec by default

Zbigniew Jędrzejewski-Szmek • 2 years ago

22804ae

rust2rpm/__main__.py

file modified

+5 -2

		`@@ -259,6 +259,9 @@`
		`return list(filter(None, (l.strip() for l in s.splitlines())))`

		`def main():`
		`+ default_target = get_default_target()`
		`+ default_rpmautospec = default_target in {"fedora"}`
		`+`
		`parser = argparse.ArgumentParser("rust2rpm",`
		`formatter_class=argparse.RawTextHelpFormatter)`
		`parser.add_argument("--show-license-map", action="store_true",`
		`@@ -270,13 +273,13 @@`
		`parser.add_argument("-", "--stdout", action="store_true",`
		`help="Print spec and patches into stdout")`
		`parser.add_argument("-t", "--target", action="store",`
		`- choices=("plain", "fedora", "mageia", "opensuse"), default=get_default_target(),`
		`+ choices=("plain", "fedora", "mageia", "opensuse"), default=default_target,`
		`help="Distribution target")`
		`parser.add_argument("-p", "--patch", action="store_true",`
		`help="Do initial patching of Cargo.toml")`
		`parser.add_argument("-s", "--store-crate", action="store_true",`
		`help="Store crate in current directory")`
		`- parser.add_argument("-a", "--rpmautospec", action="store_true", default=False,`
		`+ parser.add_argument("-a", "--rpmautospec", action="store_true", default=default_rpmautospec,`
		`help="Use autorelease and autochangelog features")`
		`parser.add_argument("--all-features", action="store_true",`
		`help="Activate all available features")`

zbyszek commented 2 years ago

rpmautspec has some shortcomings (e.g. lack of support for multi-line
changelog entries), but those shortcomings are not very important for
rust packages. Let's just use rpmautospec by default for convenience.

ngompa commented 2 years ago

:thumbsup:

Pull-Request has been merged by ngompa

2 years ago

decathorpe commented 2 years ago

I'd appreciate if I were given more than 2 hours to weigh in, since I'm basically the only pension maintaining rust packages in Fedora right now.

I'm not sure it's a good idea to enable this by default, for the simple reason that converting packages to rpmautospec is a manual process, otherwise the existing changelog is mangled and lost.

I'd be fine with enabling autospec by default if the existing target .spec file already has it enabled, but switching it on by default will result in mangled packages.

zbyszek commented 2 years ago

If the package is new — there should be no problem. If the package is old but had %autochangelog — also no problem. So the only case is when the package was had a real changelog and it's being regenerated. Should be convert to autochangelog, refuse, or keep the classic changelog?

decathorpe commented 2 years ago

Right, the transition from "non-rpmautospec" to "rpmautospec" is the only one that's problematic with defaulting to rpmautospec.

We could convert packages to rpmautospec automatically, I suppose, since that should be as simple as (pseudo-python):

spec.release = "%autorelease"`
with open("changelog", "w") as clogfile:
    clogfile.write(spec.changelog)
spec.changelog = ["%autochangelog"]

But I'm not sure if that should be the default mode of operation. I'm touching hundreds of Rust packages owned by different maintainers, and I'm not sure if all of them even want to use rpmautospec.

Additionally, I usually only convert packages to rpmautospec when there's a new Version, because for Release only bumps, it introduces weird number jumps or even downgrades in some cases, but I'm not sure if adding "is this rust2rpm run introducing a Version update compared to the last commit in origin/HEAD" code is a worthwhile effort.