Skip to content

Commit

Permalink
Remove obsolete serialization hooks from FileContentsProxy.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 371134059
  • Loading branch information
janakdr authored and copybara-github committed Apr 29, 2021
1 parent 3ea1ffc commit e4cc0b9
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.FileStatus;
import java.io.IOException;
import java.io.Serializable;
import java.util.Objects;

/**
Expand All @@ -29,15 +28,11 @@
* So if files 'a' and 'b' initially have the same timestamp, then we would think 'b' is unchanged
* after the user executes `mv a b` between two builds.
*/
public final class FileContentsProxy implements Serializable {
public final class FileContentsProxy {
private final long ctime;
private final long nodeId;

/**
* Visible for serialization / deserialization. Do not use this method, but call {@link #create}
* instead.
*/
public FileContentsProxy(long ctime, long nodeId) {
private FileContentsProxy(long ctime, long nodeId) {
this.ctime = ctime;
this.nodeId = nodeId;
}
Expand All @@ -48,16 +43,6 @@ public static FileContentsProxy create(FileStatus stat) throws IOException {
return new FileContentsProxy(stat.getLastChangeTime(), stat.getNodeId());
}

/** Visible for serialization / deserialization. Do not use this method; use {@link #equals}. */
public long getCTime() {
return ctime;
}

/** Visible for serialization / deserialization. Do not use this method; use {@link #equals}. */
public long getNodeId() {
return nodeId;
}

@Override
public boolean equals(Object other) {
if (other == this) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
package com.google.devtools.build.lib.actions;

import static com.google.common.truth.Truth.assertThat;

import com.google.common.testing.EqualsTester;
import com.google.devtools.build.lib.actions.FileContentsProxy;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.FileStatus;
import java.io.IOException;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand All @@ -26,19 +27,26 @@
@RunWith(JUnit4.class)
public class FileContentsProxyTest {
/** A simple implementation of FileStatus for testing. */
public static final class InjectedStat implements FileStatus {
private static final class InjectedStat implements FileStatus {
private final long mtime;
private final long ctime;
private final long size;
private final long nodeId;

public InjectedStat(long mtime, long ctime, long size, long nodeId) {
InjectedStat(long mtime, long ctime, long size, long nodeId) {
this.mtime = mtime;
this.ctime = ctime;
this.size = size;
this.nodeId = nodeId;
}

InjectedStat(long ctime, long nodeId) {
this.ctime = ctime;
this.nodeId = nodeId;
this.mtime = 0;
this.size = 0;
}

@Override
public boolean isFile() {
return true;
Expand Down Expand Up @@ -81,21 +89,25 @@ public long getNodeId() {
}

@Test
public void equalsAndHashCode() {
public void equalsAndHashCode() throws IOException {
new EqualsTester()
.addEqualityGroup(new FileContentsProxy(1L, 2L), new FileContentsProxy(1L, 2L))
.addEqualityGroup(new FileContentsProxy(1L, 4L))
.addEqualityGroup(new FileContentsProxy(3L, 4L))
.addEqualityGroup(new FileContentsProxy(-1L, -1L))
.addEqualityGroup(
FileContentsProxy.create(new InjectedStat(1L, 2L)),
FileContentsProxy.create(new InjectedStat(1L, 2L)))
.addEqualityGroup(FileContentsProxy.create(new InjectedStat(1L, 4L)))
.addEqualityGroup(FileContentsProxy.create(new InjectedStat(3L, 4L)))
.addEqualityGroup(FileContentsProxy.create(new InjectedStat(-1L, -1L)))
.testEquals();
}

@Test
public void fromStat() throws Exception {
public void fingerprint() throws Exception {
FileContentsProxy p1 =
FileContentsProxy.create(
new InjectedStat(/*mtime=*/1, /*ctime=*/2, /*size=*/3, /*nodeId=*/4));
assertThat(p1.getCTime()).isEqualTo(2);
assertThat(p1.getNodeId()).isEqualTo(4);
Fingerprint fingerprint = new Fingerprint();
p1.addToFingerprint(fingerprint);
assertThat(fingerprint.digestAndReset())
.isEqualTo(new Fingerprint().addLong(2L).addLong(4L).digestAndReset());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.devtools.build.lib.rules.repository;

import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.when;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.io.BaseEncoding;
Expand All @@ -27,6 +28,7 @@
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand All @@ -39,6 +41,7 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.Mockito;

/**
* Tests for @{link RepositoryFunction}
Expand Down Expand Up @@ -141,7 +144,10 @@ public void testFileValueToMarkerValue() throws Exception {
assertThat(RepositoryFunction.fileValueToMarkerValue(fv)).isEqualTo("01020304");

// Digest should also be returned if the FileStateValue doesn't have it.
fsv = new RegularFileStateValue(3, null, new FileContentsProxy(100, 200));
FileStatus status = Mockito.mock(FileStatus.class);
when(status.getLastChangeTime()).thenReturn(100L);
when(status.getNodeId()).thenReturn(200L);
fsv = new RegularFileStateValue(3, null, FileContentsProxy.create(status));
fv = new RegularFileValue(path, fsv);
String expectedDigest = BaseEncoding.base16().lowerCase().encode(path.asPath().getDigest());
assertThat(RepositoryFunction.fileValueToMarkerValue(fv)).isEqualTo(expectedDigest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,18 @@

package com.google.devtools.build.lib.skyframe;

import static org.mockito.Mockito.when;

import com.google.devtools.build.lib.actions.FileContentsProxy;
import com.google.devtools.build.lib.actions.FileStateValue;
import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.Mockito;

/** Tests for {@link FileStateValue}. */
@RunWith(JUnit4.class)
Expand All @@ -35,13 +40,20 @@ public void testCodec() throws Exception {
new FileStateValue.RegularFileStateValue(
/*size=*/ 1,
/*digest=*/ null,
new FileContentsProxy(/* ctime= */ 2, /* nodeId= */ 42)),
makeFileContentsProxy(/* ctime= */ 2, /* nodeId= */ 42)),
new FileStateValue.SpecialFileStateValue(
new FileContentsProxy(/* ctime= */ 4, /* nodeId= */ 84)),
makeFileContentsProxy(/* ctime= */ 4, /* nodeId= */ 84)),
FileStateValue.DIRECTORY_FILE_STATE_NODE,
new FileStateValue.SymlinkFileStateValue(PathFragment.create("somewhere/elses")),
FileStateValue.NONEXISTENT_FILE_STATE_NODE)
.runTests();
}

private static FileContentsProxy makeFileContentsProxy(long ctime, long nodeId)
throws IOException {
FileStatus status = Mockito.mock(FileStatus.class);
when(status.getLastChangeTime()).thenReturn(ctime);
when(status.getNodeId()).thenReturn(nodeId);
return FileContentsProxy.create(status);
}
}

0 comments on commit e4cc0b9

Please sign in to comment.