#699 [POC] *pkg import: Undo rpmautospec processing
Merged by onosek. Opened by churchyard.
churchyard/rpkg unprocess  into  master

Download 699.patch

Fixes https://pagure.io/fedpkg/issue/527

Depends-on: https://pagure.io/fedora-infra/rpmautospec/pull-request/312

This is a proof of concept, I haven't touched the tests at all. Will do that if this has a chance of being accepted.

For a long time already, I have had this idea that rpmautospec's problem with rpkg import could be solved as follows:

  1. In rpmautospec itself, implement command rpmautospec revert-process-distgit that does just that.
  2. In rpkg, do a minimal check to detect specfiles that are output from rpmautospec process-distgit. and revert them with the new command before importing.

This solution would have the good property that the code that needs detailed understanding of rpmautospec's working stays inside that project.
Also, the new command could be useful in other situations as well.

That said, having a working solution using a different approach now would be great, since

  1. I have unfortunately not been able to implement that idea yet, and many months have passed.
  2. It is not easy to get any change into rpmautospec, as opposed to rpkg where patches are always very welcome.
  3. rpkg can still switch to the new rpmautospec command later, should it ever materialize.

You pretty much summarized my thought process here as well.

Is this acceptable by the rpkg maintainers? If so, I can have a look at the tests.

The code looks good.
If this functionality will be covered in rpmautospec in the future, we can modify rpkg again.
If you want, I can take care of unittests for you.

If you want, I can take care of unittests for you.

That would be awesome, thanks!

What's the point of this range? strip_endlines is either 0 or 1. The loop will run at most once. Isn't the code equivalent to this?

if strip_endlines and lines[end+1] == "\n":
    end += 1

How many endlines should it strip?

The point of strip_endlines being a number is to allow easy adjustments of the code in case we need to strip more or less number of lines. Currently, the code is only called with 1 or 0, so it will indeed run at most once.

In the spirit of YAGNI, the ability to strip an arbitrary number of empty lines can be removed and the parameter can be changed to strip_endline=True|False. Let me know if you want me to do that.

It's indeed a trifle. If I can choose, I prefer a simpler version True|False. We can easily change it when needed.

Will do. I'm on PTO today, so perhaps tomorrow.

I am working on the tests here:
https://pagure.io/fork/onosek/rpkg/commits/undo_rpmautospec_processing?identifier=undo_rpmautospec_processing

(included the change with strip_endline)

Thanks. Here is a specfile for testing:

## START: Set by rpmautospec
## (rpmautospec version 0.3.5)
## RPMAUTOSPEC: autorelease, autochangelog
%define autorelease(e:s:pb:n) %{?-p:0.}%{lua:
    release_number = 8;
    base_release_number = tonumber(rpm.expand("%{?-b*}%{!?-b:1}"));
    print(release_number + base_release_number - 1);
}%{?-e:.%{-e*}}%{?-s:.%{-s*}}%{!?-n:%{?dist}}
## END: Set by rpmautospec
# autogenerated specfile
Summary: Dummy summary
Name: docpkg
Version: 0.2
Release: %autorelease
License: GPL
Group: Applications/Productivity
BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
Source0: hello-world.txt
Source1: docpkg.tar.gz
Source2: source-without-extension
# added empty dir just to test import srpm `fedpkg import`. It should skip the dir.
Source3: EMPTY_DIR
%description
This is a dummy description.
%prep
cp %{SOURCE0} .
%build
%clean
rm -rf $$RPM_BUILD_ROOT
%install
rm -rf $RPM_BUILD_ROOT
mkdir $RPM_BUILD_ROOT
mkdir -p $RPM_BUILD_ROOT/usr/share/doc
cp %{SOURCE0} $RPM_BUILD_ROOT/usr/share/doc/hello-world.txt
%files
%doc "/usr/share/doc/hello-world.txt"
%changelog
## START: Generated by rpmautospec
* Sun Jan  1 2006 tester <tester@example.com> - 0.2-1
- New release 0.2-1
* Sun Jan  1 2006 John Doe <jdoe@example.com> - 0.2-1
- Initial version
## END: Generated by rpmautospec

2 new commits added

  • *pkg import: Undo rpmautospec processing
  • `*pkg import: Don't delete changelog generated byrpmautospec convert```

Anyway, I took the changes wrt strip_endline from your branch, removed an unrelated style difference in a different function, tweaked things a little, and amended it here.

Commit 2bd726d2 fixes this pull-request

Pull-Request has been merged by onosek

I have realized, that line Release: %autorelease will stay unchanged in the specfile after processing. Is it intentional?
When this could be released? Should we wait on https://pagure.io/fedora-infra/rpmautospec/pull-request/312 ? If yes, how long (new boundaries would appear in specfiles after their next update)?

I have realized, that line Release: %autorelease will stay unchanged in the specfile after processing. Is it intentional?

Yes. The idea is to get the spec file as it existed before the rpmautospec processing. We want to import it in distgit with the Release: %autorelease line.

When this could be released? Should we wait on https://pagure.io/fedora-infra/rpmautospec/pull-request/312 ?

Yes.

If yes, how long (new boundaries would appear in specfiles after their next update)?

The boundaries will appear in specfiles in SRPMs generated by the new rpmautospec. Ideally, this should require a new enough rpmautospec on RPM level (e.g. via Requires: (python3-rpmautospec >= 0.3.6 if python3-rpmautospec) or Conflicts: python3-rpmautospec < 0.3.6).

https://bodhi.fedoraproject.org/updates/?packages=python-rpmautospec

Metadata