Fix InstallBypassMake symlink dependencies

Host symlinks cannot use order-only dependencies because they may be
used as part of the dependency chain on a tool, and an order-only
dependency would cause the target of the symlink not to be updated.
Use regular dependencies instead.

Bug: 204136549
Fixes: 205674000
Test: TestInstallBypassMake
Change-Id: Ib3f4ee143e94d1995ec6c60d314e7c91e57cc775
This commit is contained in:
Colin Cross
2021-11-09 16:37:52 -08:00
parent 6ac9576eef
commit 64002afc56
3 changed files with 24 additions and 12 deletions

View File

@@ -473,15 +473,19 @@ func (s *makeVarsSingleton) writeInstalls(installs, symlinks []katiInstall) []by
for _, symlink := range symlinks {
fmt.Fprintf(buf, "%s:", symlink.to.String())
if symlink.from != nil {
// The symlink doesn't need updating when the target is modified, but we sometimes
// have a dependency on a symlink to a binary instead of to the binary directly, and
// the mtime of the symlink must be updated when the binary is modified, so use a
// normal dependency here instead of an order-only dependency.
fmt.Fprintf(buf, " %s", symlink.from.String())
}
for _, dep := range symlink.implicitDeps {
fmt.Fprintf(buf, " %s", dep.String())
}
if symlink.from != nil || len(symlink.orderOnlyDeps) > 0 {
if len(symlink.orderOnlyDeps) > 0 {
fmt.Fprintf(buf, " |")
}
if symlink.from != nil {
fmt.Fprintf(buf, " %s", symlink.from.String())
}
for _, dep := range symlink.orderOnlyDeps {
fmt.Fprintf(buf, " %s", dep.String())
}

View File

@@ -2953,6 +2953,10 @@ func (m *moduleContext) InstallSymlink(installPath InstallPath, name string, src
to: fullInstallPath,
})
} else {
// The symlink doesn't need updating when the target is modified, but we sometimes
// have a dependency on a symlink to a binary instead of to the binary directly, and
// the mtime of the symlink must be updated when the binary is modified, so use a
// normal dependency here instead of an order-only dependency.
m.Build(pctx, BuildParams{
Rule: Symlink,
Description: "install symlink " + fullInstallPath.Base(),

View File

@@ -425,7 +425,8 @@ func TestInstall(t *testing.T) {
)
assertOrderOnlys(hostInstallRule("foo"))
// Check host symlink rule dependencies
// Check host symlink rule dependencies. Host symlinks must use a normal dependency, not an
// order-only dependency, so that the tool gets updated when the symlink is depended on.
assertInputs(hostSymlinkRule("foo"), hostInstallRule("foo").Output)
assertImplicits(hostSymlinkRule("foo"))
assertOrderOnlys(hostSymlinkRule("foo"))
@@ -442,7 +443,8 @@ func TestInstall(t *testing.T) {
symlinkRule("qux").Output,
)
// Check device symlink rule dependencies
// Check device symlink rule dependencies. Device symlinks could use an order-only dependency,
// but the current implementation uses a normal dependency.
assertInputs(symlinkRule("foo"), installRule("foo").Output)
assertImplicits(symlinkRule("foo"))
assertOrderOnlys(symlinkRule("foo"))
@@ -553,9 +555,10 @@ func TestInstallBypassMake(t *testing.T) {
)
assertOrderOnlys(hostInstallRule("foo"))
// Check host symlink rule dependencies
assertDeps(hostSymlinkRule("foo"))
assertOrderOnlys(hostSymlinkRule("foo"), hostInstallRule("foo").target)
// Check host symlink rule dependencies. Host symlinks must use a normal dependency, not an
// order-only dependency, so that the tool gets updated when the symlink is depended on.
assertDeps(hostSymlinkRule("foo"), hostInstallRule("foo").target)
assertOrderOnlys(hostSymlinkRule("foo"))
// Check device install rule dependencies
assertDeps(installRule("foo"), outputRule("foo").Output.String())
@@ -568,9 +571,10 @@ func TestInstallBypassMake(t *testing.T) {
symlinkRule("qux").target,
)
// Check device symlink rule dependencies
assertDeps(symlinkRule("foo"))
assertOrderOnlys(symlinkRule("foo"), installRule("foo").target)
// Check device symlink rule dependencies. Device symlinks could use an order-only dependency,
// but the current implementation uses a normal dependency.
assertDeps(symlinkRule("foo"), installRule("foo").target)
assertOrderOnlys(symlinkRule("foo"))
}
type installMakeRule struct {