[FIXED JENKINS-15829] Don't recreate workspace when using Repository Sharing

[blt04]

When using Repository Sharing, the working copy is always recloned (from the slave cache), which can take a long time on large repos.

This fixes the logic used to detect whether a workspace copy can be reused to support Repository Sharing.

JENKINS-15829

[cloudbees-pull-request-builder]

plugins » mercurial-plugin #71 SUCCESS
This pull request looks good

[jglick]

If this (3173ad4) is necessary then why does SharingSCMTest.basicOps pass without it? I would expect both

assertTrue(log, log.contains(" update --"));
assertFalse(log, log.contains(" clone --"));

(in SCMTestBase) to fail based on your description. Or if some more subtle condition is needed to trigger the bug, then this test case (or a new one, possibly in SharingSCMTest directly) ought to prove it.

[blt04]

I think this is because cloning a shared repository doesn't actually call clone. It calls update. See these lines.

Let me see if I can write some failing specs to go along with this.

[jglick]

Or rather share --noupdate, which would happen to match both of the assertions I mentioned. So perhaps SCMTestBase needs a protected method to assert that a given build log is either a clone or an update (at the caller’s choice), which SharingSCMTest would override.

[blt04]

EDIT: Please ignore this comment. It doesn't make sense, but I thought deleting it would be the wrong thing to do.

FYI: I think this is a testing issue. Most all tests call hg init. This creates an .hg/hgrc.

But in reality, .hg/hgrc is never created when using shared repos. Instead, these lines create the .hg folder, but does not create an .hg/hgrc file.

That explains why tests pass. The test creates the .hg/hgrc but in production the file isn't there.

I'm having trouble figuring out how to test this properly without rewriting SCMTestBase entirely. :(

[jglick]

OK, then I guess this needs to rest on your word based on manual testing for now. By inspection of the code it seems that the return value can only be affected in case jobUsesSharing so it ought to be safe enough to merge.

[blt04]

I appreciate the merge. If I can ever figure out a proper failing test, I will submit a new PR.

[jglick]

Yes, please do. Test coverage would be appreciated particularly for the corner cases like sharing mode that could easily get broken by supposedly unrelated changes.

[blt04]

FWIW, I found the problem. The command used to clone with the share extension:

hg --config extensions.share= share --noupdate /path/to/parent/cache /path/to/child/workspace

will create an .hg/hgrc file if and only if an .hg/hgrc file exists in the parent repo. On a jenkins master, the parent/cache repository (which was cloned from the server) does have .hg/hgrc. On a slave node, the parent/cache repository (which was cloned using hg bundle) does not have .hg/hgrc.

This explains why JENKINS-15829 only re-clones on slave nodes.

I think not having an .hg/hgrc file in the slave parent/cache is ok, thus I think this PR is correct. Unfortunately, I have no idea how to test this. If you have an suggestions, I'm happy to give it a try.

[jglick]

OK, I think I have it, hold on.