Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIXED JENKINS-73075] Clarify documentation of the environment variable P4_CHANGELIST #213

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

altostratous
Copy link

Description

Summary

Clarifies the behavior of the plugin when a workspace points to an unpopulated depot.

Problem

P4_CHANGELIST is set to the latest p4 changelist number across the Perforce server when the list of changelists is empty. This behavior is commented and can be inferred from the code:

    /**
     * Get the change number for the last change within the scope of the
     * workspace view. If there are no recent changes use the latest change.
     *
     * @return Perforce change
     * @throws Exception push up stack
     */
    public long getClientHead() throws Exception {
        // get last change in server, may return shelved CLs
        String latestChange = getConnection().getCounter("change");
        long latest = Long.parseLong(latestChange);
        P4Ref to = new P4ChangeRef(latest);
        long head = getClientHead(null, to);
        return (head == 0L) ? latest : head;
    } 

However, this edge case is not explicitly documented for P4_CHANGELIST at its description:

The last Perforce changelist number included in the populated workspace.

The documentation is currently silent about when the workspace is not populated.

Impact

It is likely that the users assume that P4_CHANGELIST always contains a submitted change list number if no error is raised and it is not null. It is likely to be used as a unique identifier of the code version in many places in CI/CD systems.

The latest change list, that may even end up being deleted without any submission, can propagate through the Jenkins systems and hurt the integrity or consistency of builds and their artifacts and logs. 

An explicit documentation may also help the users with understanding the content of P4_CHANGELIST when a workspace is not populated.

Example

If a workspace points to a non-existing depot and the user code uses Jenkins's checkout task with Perforce SCM, P4_CHANGELIST will contain the latest change list number in the Perforce server.

Recommended Solution

Change the documentation as follows:

The last Perforce changelist number included in the populated workspace.

The last Perforce changelist number included in the populated workspace, or the latest changelist number across the Perforce server if no changelist is found in the workspace.

Testing done

This is echoing what is already documented in code documents. It's a more specific documentation so I didn't include any tests for the existing behavior.

There are no changes in the code so the behavior will remain the same. However, the author has experienced this behavior.

-->

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes [N/A]
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue [N/A]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant