Remove references to ChildNameGenerator.beforeCreateItem

[basil]

See jenkinsci/cloudbees-folder-plugin#479. This workaround is no longer necessary as of jenkinsci/workflow-multibranch-plugin#359, so remove usages of the API here in preparation for eventually removing the API itself in jenkinsci/cloudbees-folder-plugin#479.

This PR is in draft jenkinsci/workflow-multibranch-plugin#359 is merged, released, and adopted.

Testing done

BOM run in jenkinsci/bom#4838. See self-review for a justification of why the change is safe.

[jglick]

I am afraid I have no recollection of what this code was for.

[basil]

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java b/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java
index 86d2af8..36890fe 100644
--- a/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java
+++ b/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java
@@ -248,30 +248,30 @@ public class WorkflowMultiBranchProjectTest {
         mp.scheduleBuild2(0).getFuture().get();
         assertNull(mp.getItem("master"));
 
-        sampleRepo.git("checkout", "-b", "feature");
+        sampleRepo.git("checkout", "-b", "feature/1");
         sampleRepo.write("another-Jenkinsfile", "echo(/branch=$BRANCH_NAME/); node {checkout scm; echo readFile('file')}");
         sampleRepo.git("add", "another-Jenkinsfile");
         sampleRepo.write("file", "subsequent content");
         sampleRepo.git("commit", "--all", "--message=feature-create");
-        WorkflowJob p1 = scheduleAndFindBranchProject(mp, "feature");
+        WorkflowJob p1 = scheduleAndFindBranchProject(mp, "feature/1");
         assertEquals(1, mp.getItems().size());
         r.waitUntilNoActivity();
         WorkflowRun b1 = p1.getLastBuild();
         assertEquals(1, b1.getNumber());
         r.assertLogContains("subsequent content", b1);
-        r.assertLogContains("branch=feature", b1);
+        r.assertLogContains("branch=feature/1", b1);
 
-        sampleRepo.git("checkout", "-b", "feature2");
+        sampleRepo.git("checkout", "-b", "feature/2");
         sampleRepo.write("another-Jenkinsfile", "echo(/branch=$BRANCH_NAME/); node {checkout scm; echo readFile('file').toUpperCase()}");
         sampleRepo.write("file", "alternative content");
         sampleRepo.git("commit", "--all", "--message=feature2-create");
-        WorkflowJob p2 = scheduleAndFindBranchProject(mp, "feature2");
+        WorkflowJob p2 = scheduleAndFindBranchProject(mp, "feature/2");
         assertEquals(2, mp.getItems().size());
         r.waitUntilNoActivity();
         WorkflowRun b2 = p2.getLastBuild();
         assertEquals(1, b2.getNumber());
         r.assertLogContains("ALTERNATIVE CONTENT", b2);
-        r.assertLogContains("branch=feature2", b2);
+        r.assertLogContains("branch=feature/2", b2);
     }
 
     @Issue("JENKINS-72613")

shows that this workaround is still used from MultiBranchProject:

idealNameFromItem:151, ChildNameGenerator (com.cloudbees.hudson.plugins.folder)
dirNameFromItem:248, MultiBranchProjectDescriptor$ChildNameGeneratorImpl (jenkins.branch)
dirNameFromItem:222, MultiBranchProjectDescriptor$ChildNameGeneratorImpl (jenkins.branch)
dirName:198, ChildNameGenerator (com.cloudbees.hudson.plugins.folder)
getRootDirFor:543, AbstractFolder (com.cloudbees.hudson.plugins.folder)
getRootDirFor:878, MultiBranchProject (jenkins.branch)
getRootDirFor:130, MultiBranchProject (jenkins.branch)
getRootDir:196, AbstractItem (hudson.model)
getConfigFile:391, Items (hudson.model)
getConfigFile:624, AbstractItem (hudson.model)
save:619, AbstractItem (hudson.model)
save:203, Job (hudson.model)
setDefinition:172, WorkflowJob (org.jenkinsci.plugins.workflow.job)
setBranch:66, AbstractWorkflowBranchProjectFactory (org.jenkinsci.plugins.workflow.multibranch)
newInstance:55, AbstractWorkflowBranchProjectFactory (org.jenkinsci.plugins.workflow.multibranch)
newInstance:45, AbstractWorkflowBranchProjectFactory (org.jenkinsci.plugins.workflow.multibranch)
observeNew:2109, MultiBranchProject$SCMHeadObserverImpl (jenkins.branch)
observe:2031, MultiBranchProject$SCMHeadObserverImpl (jenkins.branch)
process:357, SCMSourceRequest (jenkins.scm.api.trait)
discoverBranches:728, AbstractGitSCMSource$8 (jenkins.plugins.git)
run:632, AbstractGitSCMSource$8 (jenkins.plugins.git)
run:611, AbstractGitSCMSource$8 (jenkins.plugins.git)
doRetrieve:415, AbstractGitSCMSource (jenkins.plugins.git)
doRetrieve:352, AbstractGitSCMSource (jenkins.plugins.git)
retrieve:611, AbstractGitSCMSource (jenkins.plugins.git)
_retrieve:372, SCMSource (jenkins.scm.api)
fetch:282, SCMSource (jenkins.scm.api)
computeChildren:662, MultiBranchProject (jenkins.branch)
updateChildren:272, ComputedFolder (com.cloudbees.hudson.plugins.folder.computed)
run:171, FolderComputation (com.cloudbees.hudson.plugins.folder.computed)
run:1065, MultiBranchProject$BranchIndexing (jenkins.branch)
execute:101, ResourceController (hudson.model)
run:445, Executor (hudson.model)

[basil]

Blocked on jenkinsci/workflow-multibranch-plugin#359.

[basil]

AbstractWorkflowMultiBranchProjectFactory#doCreateProject does not attempt to save the project, and both SCMSourceObserver.ProjectObserver#completeNew and SCMSourceObserver.ProjectObserver#completeExisting use BulkChange, so idealNames is never consulted here.

[basil]

I am taking this out of draft now that jenkinsci/bom#4868 has been successfully adopted in BOM, but it still might make sense to wait a bit to allow people to upgrade workflow-multibranch before we release this incompatible change. When this is released, I will update the release notes to note that workflow-multibranch must be updated first. I have no strong preference about if, or how long, we wait.

[jglick]

We should probably wait a few weeks. My understanding is that the issue affects branch projects corresponding to unusually named branches, say fix/xxx (or I think branch names with _ are also affected), which for some users could be quite commonplace. Do you know offhand what the practical impact is if you are running an old version of workflow-multibranch and hit the affected code, which I guess means creating a new branch with such a name? Do we get some nasty effects like two child jobs being created, or (maybe worse) one Job in memory confused about where it lives on disk? It would be helpful to get a bom run with this PR plus older workflow-multibranch to see what the test failures look like.

[basil]

I see no reason why a bom run would be helpful here, as there is no automated test coverage for this scenario. Yes, when running this PR with an old version of workflow-multibranch some nasty effects are present; namely, one object in memory being confused about where it lives on disk (the first save gives it the wrong directory name, while the second save gives it the correct directory name, so it gets saved twice to two different directories). I am fine with waiting before we merge and release this to give consumers time to upgrade.

[basil]

We should probably wait a few weeks.

@jglick It's been a few months; any objections to moving forward with this?