Skip to content

Commit

Permalink
RELNOTES[NEW]: Added --relative_locations flag to the query command t…
Browse files Browse the repository at this point in the history
…o make the locations of build files relative to the workspace root with protobuf and XML outputs

Protobuf and XML outputs of query are non-deterministic. This flag is a first step to make genquery deterministic accross machine.

--
MOS_MIGRATED_REVID=88428100
  • Loading branch information
damienmg authored and hanwen committed Mar 13, 2015
1 parent a07d918 commit c498c98
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 6 deletions.
39 changes: 37 additions & 2 deletions src/main/java/com/google/devtools/build/lib/events/Location.java
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,13 @@ public LineAndColumn getEndLineAndColumn() {
* </pre>
*/
public String print() {
return printWithPath(getPath());
}

private String printWithPath(PathFragment path) {
StringBuilder buf = new StringBuilder();
if (getPath() != null) {
buf.append(getPath()).append(':');
if (path != null) {
buf.append(path).append(':');
}
LineAndColumn start = getStartLineAndColumn();
if (start == null) {
Expand All @@ -165,6 +169,37 @@ public String print() {
return buf.toString();
}

/**
* A default implementation of toString() that formats the location in the following ways based on
* the amount of information available:
*
* <pre>
* "foo.cc:23:2"
* "23:2"
* "foo.cc:char offsets 123--456"
* "char offsets 123--456"
*</pre>
*
* <p>This version replace the package's path with the relative package path. I.e., if {@code
* packagePath} is equivalent to "/absolute/path/to/workspace/pack/age" and {@code
* relativePackage} is equivalent to "pack/age" then the result for the 2nd character of the 23rd
* line of the "foo/bar.cc" file in "pack/age" would be "pack/age/foo/bar.cc:23:2" whereas with
* {@link #print()} the result would be "/absolute/path/to/workspace/pack/age/foo/bar.cc:23:2".
*
* <p>If {@code packagePath} is not a parent of the location path, then the result of this
* function is the same as the result of {@link #print()}.
*/
public String print(PathFragment packagePath, PathFragment relativePackage) {
PathFragment path = getPath();
if (path == null) {
return printWithPath(null);
} else if (path.startsWith(packagePath)) {
return printWithPath(relativePackage.getRelative(path.relativeTo(packagePath)));
} else {
return printWithPath(path);
}
}

/**
* Prints the object in a sort of reasonable way. This should never be used in user-visible
* places, only for debugging and testing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -484,4 +484,20 @@ protected static Pair<Iterable<Object>, AttributeValueSource> getAttributeValues

return Pair.of((Iterable<Object>) values, source);
}

/**
* Returns the target location, eventually stripping out the workspace path to obtain a relative
* target (stable across machines / workspaces).
*
* @param target The target to extract location from.
* @param relative Whether to return a relative path or not.
* @return the target location
*/
protected static String getLocation(Target target, boolean relative) {
Location location = target.getLocation();
return relative
? location.print(target.getPackage().getPackageDirectory().asFragment(),
target.getPackage().getNameFragment())
: location.print();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public class ProtoOutputFormatter extends OutputFormatter implements UnorderedFo
public static final String RULE_IMPLEMENTATION_HASH_ATTR_NAME = "$rule_implementation_hash";

private BinaryPredicate<Rule, Attribute> dependencyFilter;
private boolean relativeLocations = false;

protected void setDependencyFilter(QueryOptions options) {
this.dependencyFilter = OutputFormatter.getDependencyFilter(options);
Expand All @@ -92,6 +93,8 @@ public String getName() {
@Override
public void outputUnordered(QueryOptions options, Iterable<Target> result, PrintStream out)
throws IOException {
relativeLocations = options.relativeLocations;

setDependencyFilter(options);

Build.QueryResult.Builder queryResult = Build.QueryResult.newBuilder();
Expand Down Expand Up @@ -124,7 +127,7 @@ private void addTarget(Build.QueryResult.Builder queryResult, Target target) {
protected Build.Target toTargetProtoBuffer(Target target) {
Build.Target.Builder targetPb = Build.Target.newBuilder();

String location = target.getLocation().print();
String location = getLocation(target, relativeLocations);
if (target instanceof Rule) {
Rule rule = (Rule) target;
Build.Rule.Builder rulePb = Build.Rule.newBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,15 @@ public class QueryOptions extends OptionsBase {
+ "targets.")
public List<String> universeScope;

@Option(name = "relative_locations",
defaultValue = "false",
category = "query",
help = "If true, the location of BUILD files in xml and proto outputs will be relative. "
+ "By default, the location output is an absolute path and will not be consistent "
+ "across machines. You can set this option to true to have a consistent result "
+ "across machines.")
public boolean relativeLocations;

/**
* Return the current options as a set of QueryEnvironment settings.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class XmlOutputFormatter extends OutputFormatter implements OutputFormatter.Unor

private boolean xmlLineNumbers;
private boolean showDefaultValues;
private boolean relativeLocations;
private BinaryPredicate<Rule, Attribute> dependencyFilter;

@Override
Expand All @@ -67,6 +68,7 @@ public String getName() {
public void outputUnordered(QueryOptions options, Iterable<Target> result, PrintStream out) {
this.xmlLineNumbers = options.xmlLineNumbers;
this.showDefaultValues = options.xmlShowDefaultValues;
this.relativeLocations = options.relativeLocations;
this.dependencyFilter = OutputFormatter.getDependencyFilter(options);

Document doc;
Expand Down Expand Up @@ -182,7 +184,7 @@ private Element createTargetElement(Document doc, Target target) {
}

elem.setAttribute("name", target.getLabel().toString());
String location = target.getLocation().print();
String location = getLocation(target, relativeLocations);
if (!xmlLineNumbers) {
int firstColon = location.indexOf(':');
if (firstColon != -1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public abstract class EventTestTemplate {
@Before
public void setUp() throws Exception {
String message = "This is not an error message.";
path = scratch.path("/my/sample/path.txt");
path = scratch.path("/path/to/workspace/my/sample/path.txt");

location = Location.fromPathAndStartColumn(path, 21, 31, new LineAndColumn(3, 4));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

import com.google.devtools.build.lib.vfs.PathFragment;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand All @@ -33,4 +35,17 @@ public void fromFile() throws Exception {
assertNull(location.getEndLineAndColumn());
assertEquals(path + ":1", location.print());
}

@Test
public void testPrintRelative() throws Exception {
Location location = Location.fromFile(path);
assertEquals("/path/to/workspace/my/sample/path.txt:1",
location.print(new PathFragment("/some/other/path"), new PathFragment("baz")));
assertEquals("new/sample/path.txt:1",
location.print(new PathFragment("/path/to/workspace/my"), new PathFragment("new")));
assertEquals("new/path.txt:1",
location.print(new PathFragment("/path/to/workspace/my/sample"), new PathFragment("new")));
assertEquals("new:1", location.print(new PathFragment("/path/to/workspace/my/sample/path.txt"),
new PathFragment("new")));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public void collectsEvents() {
PrintingEventHandler handler = new PrintingEventHandler(EventKind.ERRORS_AND_WARNINGS);
handler.setOutErr(recordingOutErr);
handler.handle(event);
MoreAsserts.assertEqualsUnifyingLineEnds("WARNING: /my/sample/path.txt:3:4: "
MoreAsserts.assertEqualsUnifyingLineEnds("WARNING: /path/to/workspace/my/sample/path.txt:3:4: "
+ "This is not an error message.\n",
recordingOutErr.errAsLatin1());
assertThat(recordingOutErr.outAsLatin1()).isEmpty();
Expand Down

0 comments on commit c498c98

Please sign in to comment.