Closed Bug 974082 Opened 6 years ago Closed 6 years ago
Don't reset mock environments if we don't have to
Most of our Linux based builds use mock to manage a chroot environment to do the build in. Currently we always reset and reinitialize the mock environment at the beginning of each build, even if we're going to install exactly the same packages over. We should stop that! mock environments should only be reset when required. Related: we should make the packages for different build types the same as much as possible, to avoid resetting when we don't need to.
The idea here is to put a file inside the mock environment that contains a hash of the list of packages installed. When we start a build, look at the file, and if it's the same as our list of packages, we can skip re-initializing the mock environment. We still install packages in case there are new packages in the repos, but it's pretty quick (takes about 2s).
Comment on attachment 8378267 [details] [diff] [review] fastmock-.patch Review of attachment 8378267 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozharness/mozilla/mock.py @@ +35,5 @@ > > + def delete_mock_files(self, mock_target, files): > + """Delete files from the mock environment `mock_target`. `files` should > + be an iterable of 2-tuples: (src, dst). Only the dst component is > + deleted.""" What's the point in files being a tuples? Seems like this should just receive a simple iterable. @@ +137,5 @@ > + mock_root = self.get_output_from_command( > + ['mock_mozilla', '-r', mock_target, '--print-root-path']) > + package_hash_file = os.path.join(mock_root, "builds/package_list.hash") > + if os.path.exists(package_hash_file): > + old_packages = self.read_from_file(package_hash_file) s/old_packages/old_packages_hash/g? @@ +146,3 @@ > > + package_list_hash = hashlib.new('sha1') > + for p in sorted(mock_packages): This code falls back on c['mock_packages'] if mock_packages is None. It seems like you should be doing that here too - otherwise this won't work for scripts like servo_build.py, where mock_packages are defined in the config. @@ +149,5 @@ > + package_list_hash.update(p) > + package_list_hash = package_list_hash.hexdigest() > + > + did_init = True > + if old_packages != package_list_hash: We talked a lot in person about what to do about packages with dependencies. It looks like you've decided not to take them into account, and only deal with explicitly listed packages. Are you worried at all about things breaking if a dependency changes?
I think this addresses your concerns. The function signature for delete_files is to make it easy to pass in the mock_files structure, which is already a list of tuples.
Copy/Pasting an irc convo here, (note pastebins won't last very long, but theorycraft discussed is still useful) [11:32:45] Callek catlee: for mock_ init stuff, depending on if deplist is a good thing vs time investment you might consider using `repoquery --tree-requires` [11:33:02] Callek catlee: which will give you full deps and versions of said deps [11:33:34] =-= coop|mtg is now known as coop [11:33:40] Callek example output: https://callek.pastebin.mozilla.org/4338513 ** ED NOTE: $ repoquery --tree-requires gcc [11:35:37] Callek or you could do it the simple way if you want to concat a list for the hash: https://callek.pastebin.mozilla.org/4338522 ** ED NOTE: $ repoquery -R --recursive gcc [11:38:28] catlee Callek: huh, cool [11:38:37] catlee so that would need to run inside mock I think? [11:38:46] catlee or at least with the mock env's rpmdb [11:38:52] Callek catlee: yea I think so [11:39:16] Callek I haven't tested the theorycraft here, but it actually queries against the yum repo, not local installs [11:39:59] catlee yeah [11:40:07] catlee I wonder if that's worthwhile... [11:40:07] Callek there is also | --installed limit query to installed pkgs only| [11:40:22] Callek and lots of other options [11:44:00] Callek catlee: for set-in-stone docs, I'm copy-pasting the convo above to bug, I'll leave up to you/others on the "is it worthwhile" front [11:44:09] catlee sure
Attachment #8378336 - Flags: review?(bhearsum) → review+
And a bustage fix: http://hg.mozilla.org/build/mozharness/rev/eccc8fbf5f95
this is the only difference between the other 64-bit mock environments. adding libxml2 to these configs means we should avoid having to reset mock for any sequential b2g device builds.
Attachment #8382280 - Flags: review?(ryanvm)
Comment on attachment 8382280 [details] [diff] [review] add libxml2 to emulator/emulator-ics Review of attachment 8382280 [details] [diff] [review]: ----------------------------------------------------------------- Looks like only mako/config.json and emulator-jb/config.json already include libxml2. Should the other devices as well?
Attachment #8382280 - Flags: review?(ryanvm) → review+
I think all the other devices are using the 32-bit environment. The package lists don't need to be the same between the environments for this optimization to work.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.