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

fix: when resolving dependencies the branch attribute should be preferred #310

Merged

Conversation

David-Polehonski
Copy link
Contributor

When resolving package version dependencies the branch attribute of the dependency should be used to select the package version, not the branch name in the parameter unless the dependency doesn't specify a branch.

For example, my dependency below failed to resolve when building a package version because an equivalent build number (0.2.4.1) doesn't exist on branch dev when running the command:
sf package version create -p myPackage -b dev -k redacted -w 30 --code-coverage
"dependencies": [ { "package": "myPackageDependency", "versionNumber": "0.2.4.1", "branch": "build" },

I have built and linked packages locally and can verify this patch resolved the issue.

@salesforce-cla
Copy link

Thanks for the contribution! Before we can merge this, we need @David-Polehonski to sign the Salesforce Inc. Contributor License Agreement.

@David-Polehonski
Copy link
Contributor Author

CLA has now been signed.

@shetzel
Copy link
Contributor

shetzel commented Jun 2, 2023

@David-Polehonski - Thank you for the PR! I'll get this merged in but we should have a unit test for it. Should be pretty simple. I'd just copy the 'should create the package version create request with branch' test (lines 319-337 of packageVersionCreate.test.ts) and modify it to pass in a dependency.branch and verify the correct branch was passed to the packageCreateStub.

…rred

when resolving package version dependencies the branch attribute of the dependency should
be used to select the package version, not the branch name in parameter
unless dependency doesn't specify a branch.
@David-Polehonski David-Polehonski force-pushed the dlp/dependency-branch-selection branch from d950f3c to e94e753 Compare June 6, 2023 11:25
@David-Polehonski
Copy link
Contributor Author

@shetzel Thank you for the feedback; I have updated my initial commit to be signed/verified as I saw this was a missing requirement. I have also added a unit test; which I believe is as close to proving the feature works as I think I can get. Let me know what you think.

@shetzel shetzel merged commit f560e1c into forcedotcom:main Jun 6, 2023
@shetzel
Copy link
Contributor

shetzel commented Jun 6, 2023

Thanks @David-Polehonski - very much appreciate your contribution! 🎉

@avesolovksyy
Copy link

@David-Polehonski @shetzel @cristiand391
We faced with the same issue last week and suggested a bit different fix here: forcedotcom/cli#2183

Our suggestion:
const branch = (dependency.branch || dependency.branch === '') ? dependency.branch : this.options.branch;

Current fix by @David-Polehonski :
const branch = dependency.branch === '' ? this.options.branch : dependency.branch;

Our concern is that current fix introduce a wrong behaviour if branch is specified for dependency explicitly as empty, e.g.:

"dependencies": [
  {
    "package": "My Base Package",
    "versionNumber": "1.8.0.LATEST",
    "branch": ""
  }
]

In such case, if we create a package version from command line specifying branch key explicitly (e.g. --branch=myTestBranch), it should still respect empty branch dependency (according to sfdx guide), but with @David-Polehonski fix seems like it's going to be overwritten and would be looking for 'My Base Package' version dependency within myTestBranch, which is incorrect.

@David-Polehonski David-Polehonski deleted the dlp/dependency-branch-selection branch June 9, 2023 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants