[Notes] [Git][BuildStream/buildstream][tristan/cas-cleanup-improve] 2 commits: _cas/cascache.py: Cleanup directories when removing refs

[Tristan Van Berkom <gitlab mg gitlab com>]

Title: GitLab

Tristan Van Berkom pushed to branch tristan/cas-cleanup-improve at BuildStream / buildstream

Commits:

• 1f8e506d
  by Tristan Van Berkom at 2019-01-18T17:43:25Z

  _cas/cascache.py: Cleanup directories when removing refs
  
  With out this, empty directories in the refs/heads directory just
  grow unconditionally.
  

• 008cdee5
  by Tristan Van Berkom at 2019-01-18T17:43:25Z

  tests/artifactcache/expiry.py: Test refs directory cleanup
  
  Enhance the test which checks removal of the extract directories
  with an additional check that the ref directories are cleaned up
  when removing artifacts.
  

2 changed files:

• buildstream/_cas/cascache.py
• tests/artifactcache/expiry.py

Changes:

buildstream/_cas/cascache.py

---

...	...	@@ -21,6 +21,7 @@ import hashlib
21	21	import itertools
22	22	import os
23	23	import stat
	24	+import errno
24	25	import tempfile
25	26	import uuid
26	27	import contextlib
...	...	@@ -545,11 +546,7 @@ class CASCache():
545	546	def remove(self, ref, *, defer_prune=False):
546	547
547	548	# Remove cache ref
548		- refpath = self._refpath(ref)
549		- if not os.path.exists(refpath):
550		- raise CASCacheError("Could not find ref '{}'".format(ref))
551		-
552		- os.unlink(refpath)
	549	+ self._remove_ref(ref)
553	550
554	551	if not defer_prune:
555	552	pruned = self.prune()
...	...	@@ -626,6 +623,55 @@ class CASCache():
626	623	def _refpath(self, ref):
627	624	return os.path.join(self.casdir, 'refs', 'heads', ref)
628	625
	626	+ # _remove_ref()
	627	+ #
	628	+ # Removes a ref.
	629	+ #
	630	+ # This also takes care of pruning away directories which can
	631	+ # be removed after having removed the given ref.
	632	+ #
	633	+ # Args:
	634	+ # ref (str): The ref to remove
	635	+ #
	636	+ # Raises:
	637	+ # (CASCacheError): If the ref didnt exist, or a system error
	638	+ # occurred while removing it
	639	+ #
	640	+ def _remove_ref(self, ref):
	641	+
	642	+ # Remove the ref itself
	643	+ refpath = self._refpath(ref)
	644	+ try:
	645	+ os.unlink(refpath)
	646	+ except FileNotFoundError:
	647	+ raise CASCacheError("Could not find ref '{}'".format(ref)) from e
	648	+
	649	+ basedir = os.path.join(self.casdir, 'refs', 'heads')
	650	+ components = list(os.path.split(ref))
	651	+
	652	+ while components:
	653	+ components.pop()
	654	+ refdir = os.path.join(basedir, *components)
	655	+
	656	+ # Break out once we reach the base
	657	+ if refdir == basedir:
	658	+ break
	659	+
	660	+ try:
	661	+ os.rmdir(refdir)
	662	+ except FileNotFoundError:
	663	+ # The parent directory did not exist, but it's
	664	+ # parent directory might still be ready to prune
	665	+ pass
	666	+ except OSError as e:
	667	+ if e.errno == errno.ENOTEMPTY:
	668	+ # The parent directory was not empty, so we
	669	+ # cannot prune directories beyond this point
	670	+ break
	671	+
	672	+ # Something went wrong here
	673	+ raise CASCacheError("System error while removing ref '{}': {}".format(ref, e)) from e
	674	+
629	675	# _commit_directory():
630	676	#
631	677	# Adds local directory to content addressable store.

tests/artifactcache/expiry.py

---

...	...	@@ -382,6 +382,7 @@ def test_extract_expiry(cli, datafiles, tmpdir):
382	382	res = cli.run(project=project, args=['checkout', 'target.bst', os.path.join(str(tmpdir), 'checkout')])
383	383	res.assert_success()
384	384
	385	+ # Get a snapshot of the extracts in advance
385	386	extractdir = os.path.join(project, 'cache', 'artifacts', 'extract', 'test', 'target')
386	387	extracts = os.listdir(extractdir)
387	388	assert(len(extracts) == 1)
...	...	@@ -395,3 +396,16 @@ def test_extract_expiry(cli, datafiles, tmpdir):
395	396
396	397	# Now the extract should be removed.
397	398	assert not os.path.exists(extract)
	399	+
	400	+ # As an added bonus, let's ensure that no directories have been left behind
	401	+ #
	402	+ # Now we should have a directory for the cached target2.bst, which
	403	+ # replaced target.bst in the cache, we should not have a directory
	404	+ # for the target.bst
	405	+ refsdir = os.path.join(project, 'cache', 'artifacts', 'cas', 'refs', 'heads')
	406	+ refsdirtest = os.path.join(refsdir, 'test')
	407	+ refsdirtarget = os.path.join(refsdirtest, 'target')
	408	+ refsdirtarget2 = os.path.join(refsdirtest, 'target2')
	409	+
	410	+ assert os.path.isdir(refsdirtarget2)
	411	+ assert not os.path.exists(refsdirtarget)

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.