forcedotcom / cli

Salesforce CLI
https://developer.salesforce.com/docs/atlas.en-us.sfdx_cli_reference.meta/sfdx_cli_reference/
BSD 3-Clause "New" or "Revised" License
488 stars 78 forks source link

force:source:beta:push does not support MPD #1269

Closed kjonescertinia closed 2 years ago

kjonescertinia commented 2 years ago

Summary

If you use force:source:beta:push on a project with multiple package directories it appears to try push all the metadata in one go which can result in INVALID_OPERATION: Too many files in zip errors and likely other errors due to conflicts between the package directory metadata which has been designed to work with MPD.

This may just be an oversight in the known issues list, I am raising it because it was not mentioned on #1258.

Steps To Reproduce:

From a 1GP Project using multiple package directories run:

  1. Create a new scratch org
  2. sfdx force:source:beta:push -f

Expected result

There are multiple deploys to the org, each one corresponding to a single package directory in sfdx-project.json.

Actual result

There is a single push.

System Information

{ "cliVersion": "sfdx-cli/7.125.0", "architecture": "darwin-x64", "nodeVersion": "node-v12.22.7", "pluginVersions": [ "@oclif/plugin-autocomplete 0.3.0 (core)", "@oclif/plugin-commands 1.3.0 (core)", "@oclif/plugin-help 3.2.3 (core)", "@oclif/plugin-not-found 1.2.4 (core)", "@oclif/plugin-plugins 1.10.1 (core)", "@oclif/plugin-update 1.5.0 (core)", "@oclif/plugin-warn-if-update-available 1.7.0 (core)", "@oclif/plugin-which 1.0.3 (core)", "@salesforce/sfdx-plugin-lwc-test 0.1.7 (core)", "alias 1.1.22 (core)", "apex 0.3.0 (core)", "auth 1.7.5 (core)", "config 1.2.41 (core)", "custom-metadata 1.0.12 (core)", "data 0.6.4 (core)", "generator 1.2.1 (core)", "limits 1.2.2 (core)", "org 1.9.0 (core)", "salesforce-alm 53.1.2 (core)", "schema 1.0.10 (core)", "sfdx-cli 7.125.0 (core)", "source 1.3.1 (core)", "telemetry 1.2.8 (core)", "templates 52.4.0 (core)", "trust 1.0.11 (core)", "user 1.5.2 (core)" ], "osVersion": "Darwin 20.6.0" }

github-actions[bot] commented 2 years ago

Thank you for filing this issue. We appreciate your feedback and will review the issue as soon as possible. Remember, however, that GitHub isn't a mechanism for receiving support under any agreement or SLA. If you require immediate assistance, contact Salesforce Customer Support.

mshanemc commented 2 years ago

This is the expected behavior...the new library intelligently combines MPD into one deploy. That's also how source:deploy currently works.

Do you have a way to reproduce one of the potential errors so we can make sure they don't occur? Just a 10k+ files error?

kjonescertinia commented 2 years ago

One I hit was duplicate labels (different labels files), under MPD this is OK but it failed with beta push at the end of the deploy. I can't see others because of the 10k issue, but we will have issues with duplicate classes as well.

This direction generally worries me, converting large products to MPD was expensive. Is there going to be somewhere I can point people to that lays out what work is going to be needed to adopt this way of working?

azlam-abdulsalam commented 2 years ago

@mshanemc I have raised an issue on this as well, https://github.com/forcedotcom/cli/issues/1265

As @kjonesffdc has mentioned, having MPD is a strong requirement when you are in a monorepo with both unlocked packages as well as mdapi packages, (size is one factor), also there are behaviours such as overrides for unsupported attributes and org specific entities which have to be sequenced.

And does this also means, until the entire repo is good we wont be able to get a partial org, as the whole metadata either deploys or not, resulting in longer feedback for devs

mshanemc commented 2 years ago

I added it to the beta's known issues section to make people aware. [I couldn't tell from #1265 what the actual problem is]

While we're here...can I get some feedback on how you'd like this to work?

Option 1

force:source:push --pkgDir some/path That way, you could deploy them in whatever order makes sense, or repeat the push of a pkgDir with some changes (if, for example, you pushed the first 3 and they were good but had an error in the 3rd)

Option 2

force:source:push --sequential basically, tell it to do multiple deployments in the order specified by the pkgDirs

Option 3

force:source:deploy -p some/path --tracking where this causes a deployment but

  1. checks for conflicts before pushing
  2. updates the tracking files with the deploy results after a successful deploy
uip-robot-zz commented 2 years ago

This issue has been linked to a new work item: W-10146239

kjonescertinia commented 2 years ago

Would it be possible to add something like the sequential flag into sfdx-project.json as it's really declaring how to interpret the package directory contents, e.g. duplicate rules are more lax when it is set. I am thinking if this is not visible for other tools to read it is going to make it hard for them to operate correctly.

azlam-abdulsalam commented 2 years ago

@kjonesffdc I like that idea as well

mshanemc commented 2 years ago

@kjonesffdc what's happening with the duplicate labels? That sounds more like a bug. Is it like

pkgDir1/labels/CustomLabels.labels-meta.xml pkgDir2/labels/CustomLabels.labels-meta.xml

where both files have an identical label?

And are you able to reproduce an error with duplicate classes, or just expect that you would? And are they the same class exactly, or have different content on the same name?

And how does source:pull work with this, anyway? Does it know where to put changes?

azlam-abdulsalam commented 2 years ago

@mshanemc for us as its not split, typically it has the different content in different files. We use kind of a psuedo namespacing to replace it after a pull

kjonescertinia commented 2 years ago

@mshanemc Looks like I might have messed up testing the labels, I had swapped to single package directory so not sure if that should have given an error earlier or if you are relying on deploy to pick up duplicates. In re-testing I am also seeing missing labels in the deploy if I use SFDX_MDAPI_TEMP_DIR, only second package directory labels appear to be being pushed.

To avoid further confusion I will simplify test case and create a repo that I can share it.

nawforce commented 2 years ago

Reproduce of the duplicate label problem is at https://github.com/nawforce/sfdx-test/tree/main/duplabels. For classes have a look at https://github.com/nawforce/sfdx-test/tree/main/dupclasses. It looks like class deploy order may be dependent on the sort order of the package directory itself, it's OK if the package directories are ordered one way but fails if they are reversed.

I don't use source:pull, everything is pushed in normal operations so I default to 'push -f', sometimes I need to retrieve from customer orgs but its pretty rare. The duplicates are fallout from needing to partition from MPD at short notice, they are slowly being removed but as MPD supports 'deploy order' it's not been a high priority.

mshanemc commented 2 years ago

@nawforce I'm curious what you're using push for if you're not doing source tracking operations (push/pull).

What does push give you that source:deploy wouldn't?

azlam-abdulsalam commented 2 years ago

@mshanemc I did further testing on these commands on a mid sized repo. Here are some observations

  1. source:beta:push detects the total component as 1745 components
  2. source:push's final updates mentions it at 1724 components (though actual components pushed is 1727)
  3. From a time of deployment perspective from the first deployment to the final one ~27 mins, where as without mpd ~14 mins, till it reported a failure
  4. Please find the below errors surfaced so far, the same repo works perfectly fine with MPD. Probably due to the order in which object defintion files are deployed before the rules etc. This is what I am basically worried, as there will be lot of regression across many projects (though fixable)

Screen Shot 2021-11-14 at 8 26 14 am

kjonescertinia commented 2 years ago

I was looking at push as it's the most commonly used command for its local change detection capabilities, so problems with it effect DX more than anything else we use from the CLI (although org creation failures are a close second). We have been stuck on 7.75 for nearly a year now due to one issue or another, so really I am trying to work out how we get to a happier place and if the beta commands help with that.

mshanemc commented 2 years ago

what's keeping you on 7.75? @kjonesffdc

mshanemc commented 2 years ago

@azlam-abdulsalam we're gonna make some option for you to push packageDirs sequentially before this goes live, and keep the old version of push around under a force:source:legacy subtopic for a while in case anyone still needs it.

mshanemc commented 2 years ago

@kjonesffdc @azlam-abdulsalam sequential deployments are now available, pretty much as you wanted it. See the updated beta notes for how this works now (note--that's available starting in the 7.130.1, currently latest-rc)

kjonescertinia commented 2 years ago

Thanks for the quick turnaround on this, I will get setup with the latest and continue testing.

kjonescertinia commented 2 years ago

From what I can see main issue stopping us upgrading recently has been https://github.com/forcedotcom/salesforcedx-vscode/issues/3541, looks like that should be resolved soon.

mshanemc commented 2 years ago

closed with new commands GA