[cipd] make creating instances from cipd installations work.

[cipd] make creating instances from cipd installations work.

R=vadimsh@chromium.org
BUG=689359
Review-Url: https://codereview.chromium.org/2681803003
Committed: https://github.com/luci/luci-go/commit/9713e46074c62af9469ada39da61164274c3295f

[Vadim Sh., 2017-02-08 02:46:51]

https://codereview.chromium.org/2681803003/diff/1/cipd/client/cipd/local/file...
File cipd/client/cipd/local/files.go (right):

https://codereview.chromium.org/2681803003/diff/1/cipd/client/cipd/local/file...
cipd/client/cipd/local/files.go:141: if root == dir && filepath.Join(root,
SiteServiceDir) == abs {
I think "root == dir" condition is unnecessary

https://codereview.chromium.org/2681803003/diff/1/cipd/client/cipd/local/file...
cipd/client/cipd/local/files.go:227: // files within SiteServiceDir get resolved
as normal files.
"Symlinks with targets within ...."

https://codereview.chromium.org/2681803003/diff/1/cipd/client/cipd/local/file...
cipd/client/cipd/local/files.go:228: if strings.HasPrefix(target,
SiteServiceDir+string(os.PathSeparator)) {
filepath.Separator, like in the rest of this file

https://codereview.chromium.org/2681803003/diff/1/cipd/client/cipd/local/file...
cipd/client/cipd/local/files.go:228: if strings.HasPrefix(target,
SiteServiceDir+string(os.PathSeparator)) {
target can be relative and will not start with SiteServiceDir. Resolve it to
absolute path and use isSubpath(...) function

[iannucci, 2017-02-08 02:53:49]

https://codereview.chromium.org/2681803003/diff/1/cipd/client/cipd/local/file...
File cipd/client/cipd/local/files.go (right):

https://codereview.chromium.org/2681803003/diff/1/cipd/client/cipd/local/file...
cipd/client/cipd/local/files.go:141: if root == dir && filepath.Join(root,
SiteServiceDir) == abs {
On 2017/02/08 02:46:51, Vadim Sh. wrote:
> I think "root == dir" condition is unnecessary

you're right (there's a better way)

https://codereview.chromium.org/2681803003/diff/1/cipd/client/cipd/local/file...
cipd/client/cipd/local/files.go:228: if strings.HasPrefix(target,
SiteServiceDir+string(os.PathSeparator)) {
On 2017/02/08 02:46:51, Vadim Sh. wrote:
> filepath.Separator, like in the rest of this file

Done.

[Vadim Sh., 2017-02-08 02:54:55]

https://codereview.chromium.org/2681803003/diff/20001/cipd/client/cipd/local/...
File cipd/client/cipd/local/files.go (right):

https://codereview.chromium.org/2681803003/diff/20001/cipd/client/cipd/local/...
cipd/client/cipd/local/files.go:232: info, err = os.Stat(abs)
nit: targetAbs, maybe?.. So save os.Stat trouble following the symlink, because
we already did :)

[iannucci, 2017-02-08 05:09:41]

Added some tests, manually verified that it works too :)

[Vadim Sh., 2017-02-08 05:24:38]

lgtm

[iannucci, 2017-02-08 07:58:00]

The CQ bit was checked by iannucci@chromium.org

[commit-bot: I haz the power, 2017-02-08 07:58:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 08:08:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 08:08:42]

Try jobs failed on following builders:
  Luci-go Win Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/34358c704d0f2a10)

[iannucci, 2017-02-08 08:12:20]

The CQ bit was checked by iannucci@chromium.org

[iannucci, 2017-02-08 08:12:20]

The patchset sent to the CQ was uploaded after l-g-t-m from vadimsh@chromium.org
Link to the patchset: https://codereview.chromium.org/2681803003/#ps60001
(title: "windows tests")

[commit-bot: I haz the power, 2017-02-08 08:12:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 08:19:18]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1486541540489540,
"parent_rev": "48cb64fb084da1ea3d01268c1845fd60f64f5c1c", "commit_rev":
"9713e46074c62af9469ada39da61164274c3295f"}

[commit-bot: I haz the power, 2017-02-08 08:19:21]

Description was changed from

==========
[cipd] make creating instances from cipd installations work.

R=vadimsh@chromium.org
BUG=689359
==========

to

==========
[cipd] make creating instances from cipd installations work.

R=vadimsh@chromium.org
BUG=689359

Review-Url: https://codereview.chromium.org/2681803003
Committed:
https://github.com/luci/luci-go/commit/9713e46074c62af9469ada39da61164274c3295f
==========

[commit-bot: I haz the power, 2017-02-08 08:19:22]

Committed patchset #4 (id:60001) as
https://github.com/luci/luci-go/commit/9713e46074c62af9469ada39da61164274c3295f