For consistency, a variable should always include both the account/organization and the repository name, even in the normal case that the fork has the same name as the original repository. Since CHANGE_FORK has been only an account/organization for over two years now, we would risk breaking existing scripts by changing its meaning now. Would need to introduce a new variable name, which means

• (scm-api) introducing String SCMHeadOrigin.Fork.getFullName(), I guess defaulting to null
• (branch-api) making BranchNameContributor bind it when set, say to CHANGE_FORK_FULL
• (github-branch-source et al.) overriding the new method in, e.g., PullRequestSCMHead

I've got a bit skeptical about the backward compatibility, as the current implementations for already provides the account|org/repo format:

• Bitbucket (https://github.com/jenkinsci/bitbucket-branch-source-plugin/blob/34c73ea3aeff54bc037e6f6caaffe1a7c86b6c29/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java#L1146)
• Gitlab (https://github.com/jenkinsci/gitlab-branch-source-plugin/blob/6478e861e1b257f1e64c63db18814b578eb2d0c6/src/main/java/io/jenkins/plugins/gitlabbranchsource/GitLabMergeRequestSCMCommentEvent.java#L52-L53)

The proposal of creating a new env variable will provide a misleading since CHANGE_FORK and CHANGE_FORK_FULL env variables will behave differently based on the specific branch source plugin and therefore might confuse. Besides, the API is the one which specifies what it should be done, therefore the implementation should proceed with the requirement.

In other words:

• github-branch-source  will contain two env variables: CHANGE_FORK=account CHANGE_FORK_FULL=account/repo
• bitbucket-branch-source: CHANGE_FORK=account/repo CHANGE_FORK_FULL=account/repo
• gitlab-branch-source: CHANGE_FORK=account/repo CHANGE_FORK_FULL=account/repo

As the implementation is not right, it means it's required to touch even in the API to solve it which it might be awkward. For sure it's a breaking change, but people should already be aware the implementation itself should be account/repo as already noticed in the docs. So they should not be surprised about it, although adding an entry in the release notes with the implication of those changes might be enough.

Besides, I've got a question, the current implementation for  CHANGE_FORK is invalid when the fork name differs from the upstream name, therefore how do we encourage people to avoid using it and use CHANGE_FORK_FULL? 

jglick

As I've noted, PR-235 is not perfect.  I agree with you that the right solution in the long term is involves fixing the underlying API/data.

However, in the short term I'm inclined to take PR-235 and open a new JIRA to address the API issue when time permits.  

I do have concerns as noted below, but I don't think they are sufficient to block us for taking PR-235. 

Pros

• Solves definite flaw in the way the plugin exposes fork information (it is not really sufficient in all reasonable cases)
• Use case is uncommon but not exceptional - It is uncommon for fork repos to have different names from their roots, but it is valid and not that rare.
• Does not break existing cases (when repo names are the same)
• Changes the data stored in underlying object not just the exposed CHANGE_FORK - all consumers of origin will see the same information.
• Change is contained to this plugin and brings the behavior into accordance with the API as described here:
  https://github.com/jenkinsci/scm-api-plugin/blob/5b5e58e70489167414b77fd1a7e8a92bf7b3e68e/src/main/java/jenkins/scm/api/SCMHeadOrigin.java#L129-L152

Concerns

• New value format changes depending on forked repo name - old format is always owner-name, new format is owner-name/forked-repo-name, but only when the the fork repo name differs from the root.  Consumers of CHANGE_FORK must now check if it contains a slash to know which format to use.
• Changes the data stored in underlying object not just the exposed CHANGE_FORK - despite this being valid based on the API description, all consumer of origin will be effected and will need to be tested as correctly handling this change.

Related notes:
CHANGE_FORK is exposed here:
https://github.com/jenkinsci/branch-api-plugin/blob/c4d394415cf25b6890855a08360119313f1330d2/src/main/java/jenkins/branch/BranchNameContributor.java#L72-L74

I had missed that the Javadoc actually encourages an implementation to switch formats, meaning the current PR follows the API, as confusing and unfriendly as this is. I do not know enough about BitBucket or GitLab to comment on their behaviors. In light of all this, I guess the PR’s behavior is tolerable enough, though it will certainly merit a release note, and https://github.com/jenkinsci/branch-api-plugin/blob/db3c6487e3fa9f0214b6e50e5808eba84b531aff/src/main/resources/jenkins/branch/BranchNameContributor/buildEnv.properties#L32 must be corrected to explain that the specific format may not only vary from one SCM/provider to the next, but may vary even across different cases in a single provider.

JENKINS-58716 can I think work with a variable format, though it is not ideal: use the full string if it contains a / and otherwise append the inferred repository name.

Thanks jglick.  I've merged.  This definitely needs a release note.  It think it deserves a minor version bump as well.