Bug 1392137 - Don't delete generated RPMs when cleaning chroot
Summary: Don't delete generated RPMs when cleaning chroot
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: mock
Version: rawhide
Hardware: Unspecified
OS: Linux
low
low
Target Milestone: ---
Assignee: Clark Williams
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-11-05 03:32 UTC by Audrey Yeena Toskin
Modified: 2017-04-25 16:13 UTC (History)
7 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2017-04-25 16:13:59 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Audrey Yeena Toskin 2016-11-05 03:32:22 UTC
Continuing an issue I started in the middle of #1312677.

Creating a binary RPM means running `mock --buildsrpm ...` and then `mock --rebuild ...`. Both commands by default place source or binary RPMs under /var/lib/mock/, *after* deleting everything in there first. Meaning that `--rebuild` will by default delete the SRPM created in the previous step. mock's current default behavior requires the user to intervene by either:

1. adding `--no-clean` to the rebuild command
2. adding `--resultdir` to the build SRPM command, or
3. manually moving the generated SRPM between mock commands.

Each of these are less-than-ideal. For instance, to ensure a consistent build, there's good reason to clean up the chroot every time, right? And specifying an output directory elsewhere in the filesystem, or manually moving the source RPM, is one more thing for the RPM spec author to have to think about when running builds. It would be better if mock's own default behavior weren't so self-destructive.

Placing the generated source RPMs in such a volatile directory and then, when building the binary RPMs, requiring the user to go out of their way to prevent mock from destroying its own prior work is, in my opinion, bad design, bad user experience, and annoying. Wouldn't it be better if the default location for the generated RPMs were a little safer? Perhaps when cleaning the chroot, you could spare the results directory. Or choose a new default directory to store results, somewhere where the newly created RPMs will be unaffected by the chroot cleanup.

Comment 1 Miroslav Suchý 2016-11-07 08:25:28 UTC
I do not think that semantic of /var/lib/mock/ will change. This is basicaly working directory for mock. It is not place for you to store there build artefacts.

Note that you you can put in config:

# if you want mock to backup the contents of a result dir before clean
# config_opts['backup_on_clean'] = False
# config_opts['backup_base_dir'] = "/your/backup/dir"

And you can execute any command after build. See
  https://github.com/rpm-software-management/mock/wiki/Plugin-Sign
So you can put something like:

config_opts['plugin_conf']['sign_enable'] = True
config_opts['plugin_conf']['sign_opts'] = {}
config_opts['plugin_conf']['sign_opts']['cmd'] = 'cp'
config_opts['plugin_conf']['sign_opts']['opts'] = '-a %(rpms)s /your/backup/dir'

Ans you can use --resultdir (which you can put in ~/.config/mock.cfg) permanently.

So you have at least 3 ways how to solve your problem.

Comment 2 Audrey Yeena Toskin 2016-11-08 10:11:13 UTC
Right, my point wasn't so much about the lack of possible solutions. And I think mock having its own working directory under /var/ is perfectly fine and good.

I was rather trying to point out that mock uses silly defaults. mock creates RPMs which can either be installed directly or submitted to Fedora packaging infrastructure; however, you can't install or inspect or lint or submit those RPMs if mock destroys them. You also can't build the binary RPM if mock destroys the source RPM. Saving and using the generated RPMs is, I should think, a sufficiently common use case that it would make more sense if mock placed RPMs someplace safe *by default*. /var/lib/mock/ can continue to exist and be used as it always has -- *if* the RPMs are spared, or stored outside that workspace.

The value in designing a program's defaults to ease the most common uses seems self-evident to me. Am I missing something that would make the current design make more sense?

Comment 3 Miroslav Suchý 2016-11-08 11:57:57 UTC
Let's dive deeper.

Lets build package glibc.src.rpm:
 mock -r fedora-24-x86_64 glibc.src.rpm
Mock prepares chroot in /var/lib/mock/fedora-24-x86_64/root/, run `rpmbuild` there, which will put results in /var/lib/mock/fedora-24-x86_64/root/builddir/build/RPMS/  (you can verify if you run mock with --no-clean).
Mock copy resulting RPMS to resultdir. By default:
  /var/lib/mock/fedora-24-x86_64/result
See, we already copy result to "safe" directory. But in that directory can be previous results. For example glibc package has dozens of subpackage. And e.g. it is not easy to guess that "nscd" is actually subpackage of glibc, because it does not have glibc- prefix. So you either have to wipe resultdir before putting results there, or users will have hard time what actually the result is.

And having default, which will copy the results to *yet another* directory, which is not wiped before results are put there, really does not make sense to me.

Comment 4 Audrey Yeena Toskin 2016-11-11 01:25:45 UTC
I feel like I'm repeating myself, but I'm also not sure my point is really getting across.

> Mock copies resulting RPMS to resultdir. By default:
>   /var/lib/mock/fedora-24-x86_64/result
> See, we already copy result to "safe" directory.

The "result" directory *isn't* safe, though, if you wipe it every time. So the user is forced to figure out some other place to put their RPMs anyway.

> But in that directory can be previous results. For example glibc package has dozens of subpackage. And e.g. it is not easy to guess that "nscd" is actually subpackage of glibc, because it does not have glibc- prefix. So you either have to wipe resultdir before putting results there, or users will have hard time what actually the result is.

Those subpackages have to get built one way or another. Hopefully, the package maintainer should know to expect those subpackages, but even if they didn't, wiping the results directory isn't going to help with confusion over subpackages. So I don't see how that example is relevant.

Maybe we could think of it like this:

What are the use cases for wanting to redo a build? Perhaps the previous build failed, or the user needed to add or change metadata. In such cases, *overwriting* previous builds seems like a good assumption, because the user likely wouldn't run a build again if their previous results were already perfect. So in some cases it's probably okay to touch the RPMs generated in a previous iteration.

On the other hand, consider the way building a binary RPM works. It requires a source RPM as input. The source RPM by default goes in the same results directory as the binary RPM -- which by default gets wiped when building the binary RPM. So even if the source and binary builds would normally succeed, `mock --rebuild` will *always fail* because it destroys the results of `mock --buildsrpm`, unless the user goes out of their way to save the source RPM between these two steps.

I'm not necessarily saying mock results needs some other directory outside of /var/lib/mock/. And I'm NOT saying mock must never erase data.

I AM saying that `--buildsrpm` should probably avoid erasing binary RPMs, and `--rebuild` should *never* erase the source RPMs. Otherwise, we have the current problem where `--rebuild` destroys its input SRPM before it can even use it. The `--buildsrpm` and `--rebuild` subcommands should be smart enough to avoid interfering with each other's work.

Does that make sense?

Comment 5 Miroslav Suchý 2016-11-11 09:21:22 UTC
> I AM saying that `--buildsrpm` should probably avoid erasing binary RPMs, and `--rebuild` should *never* erase the source RPMs.

Definitely not true. The resulting SRPM is rebuild in chroot. So e.g. submitted SRPM can be build on some recent Fedora, but
   mock --rebuildsrpm -r epel-5-i386 foo.src.rpm 
can produce SRPM which is little bit different (e.g. can have different checksum - sha vs. md5). So keeping the original SRPM in result dir and not overwriting it will confuse a lot of people.

Comment 6 Audrey Yeena Toskin 2016-11-11 21:18:34 UTC
> So keeping the original SRPM in result dir and not overwriting it will confuse a lot of people.

Again, like I said, I think overwriting previous results when redoing a specific build is fine and good. It may even be okay to erase binary results during a source build, because then the user is basically resetting the whole process. The problem is when building a binary RPM erases the source RPMs, because the binary build *needs* the SRPM as input. With the current behavior, binary builds in mock are broken by default.

Users are required to manually move source RPMs or add extra flags so that they can be used in binary builds, which I think is incredibly silly. *That* is all I'm asking the mock developers to fix.

Comment 7 Vit Ry 2017-02-27 10:20:56 UTC
I just using smth like

rd=$(mktemp -d)
mock --resultdir $rd my.src.rpm

mv $rd/*.log buildnumber/logs/
mv $rd/*.src.rpm /repo/src
mv $rd/*.rpm /repo/

and it works just as expected...
In any case user needs some build-script around mock.

I think, you should think 'results are lost / it's just test', in case you did not specify clean resultdir manually.

Comment 8 Miroslav Suchý 2017-04-25 16:13:59 UTC
> The problem is when building a binary RPM erases the source RPMs, because the binary build *needs* the SRPM as input.

Once again. When you put input files into directory where mock puts output you are breaking mock workflow. If we do not delete result_dir it would break some things (e.g. sign plugin).


Note You need to log in before you can comment on or make changes to this bug.