diff --git a/android/makevars.go b/android/makevars.go index 7d8864ac6..8825d1aa6 100644 --- a/android/makevars.go +++ b/android/makevars.go @@ -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()) } diff --git a/android/module.go b/android/module.go index 3447f2b51..c778078d0 100644 --- a/android/module.go +++ b/android/module.go @@ -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(), diff --git a/android/module_test.go b/android/module_test.go index 236cae79a..8607a8d34 100644 --- a/android/module_test.go +++ b/android/module_test.go @@ -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 {