Skip to content
This repository has been archived by the owner on Feb 4, 2020. It is now read-only.

Feature/add source dir #309

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MikeRogers27
Copy link

This modification adds a new base directory for source code that can be used to achieve path independence if your build and source are under different roots.

Copy link
Owner

@frerich frerich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Git commit log doesn't follow the standard format, and it's not terribly informative. Please see https://github.com/frerich/clcache/wiki/Contributing#commit-logs for how to write commit logs - in particular, it would be great if you could explain the scenario in which this patch helps, how the behaviour is without the patch and how the behaviour is with the patch.

additionalData = "{}|{}|{}".format(
compilerHash, commandLine, ManifestRepository.MANIFEST_FILE_FORMAT_VERSION)
printTraceStatement("Hashing additionalData: {}".format(additionalData))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two printTraceStatement statements seem unrelated to the commit - can you move them into a separate commit?

clcache.py Outdated
return path.replace(baseDir, BASEDIR_REPLACEMENT, 1)
if not sourceDir is None:
assert sourceDir == os.path.normcase(sourceDir)
if path.startswith(sourceDir):
return path.replace(sourceDir, SOURCEDIR_REPLACEMENT, 1)
return path
return path
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code uses an indentation of four spaces per level. Let's stick to that format (if you like, you can merge this commit into the previous commit while you're at it).

@frerich
Copy link
Owner

frerich commented Apr 17, 2018

I'm not totally sure I understand the feature yet - can you please add some tests (e.g. to tests_integration.py) which demonstrate how the patch helps?

@ghost
Copy link

ghost commented Jul 11, 2018

what's the difference between CLCACHE_BASEDIR and CLCACHE_SOURCEDIR?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants