handle local file rename/moves

[mshanemc]

we had issues open around renames and moves

1. Deploy fails when case of file name is changed #1362
2. SFDX Push error when metadata is moved #2035

and it comes up sometimes in conversations.

The CLI hasn't supported this well because the mdapi and source tracking don't have a clean way to indicate renamed/moved files. (to the server it's a delete of the first one and a create of the second).

Let's have a discussion so y'all can comment and upvote if this is a big deal to you vs other priorities and do some solutioning

[mshanemc]

case 1: reorganizing

files/dirs between project directories (ex: force-app/main/default/tabs/Foo__c => dir2/tabs/Foo__c)

Because the org doesn't require changing, we'd detect that as a "move" and not count it as something that needs to be deployed/deleted. We'd be looking for deleted files that have the same sha as a created file, and we'd omit those from the changes (basically doing a commit to the local git repo to "clear" those changes.

This would solve #2035

Vote this comment up if the use case is important to you.

[mshanemc]

Isn't part of the SFDX best practice to never have metadata in the "default" folder, so after a retrieve the metadata must always be moved into a carefully selected alternative folder?

that's new to me. What's the advantage of doing that?

[petter-eikeland]

If you maintain your metadata in a single repository, especially with packaging, you need to reorganize all new metadata you retrieve. sf has no way of knowing where this new metadata fits in your repository structure, other than placing it in the "default" folder. Hence, you need to make a conscious decision of which directory/package to place the metadata.

Of course, you could still use the default directory, however in most cases you still need to move it from the default one and into a different packaged directory.

[VivekMChawla]

@mshanemc - From the beginning of 2GP, I've promoted, as best practice, the idea of having an unsorted directory set as the default package directory for all MPSN (multi-package, same-namespace) projects.

This ensures that any metadata created declaratively and subsequently pulled into the local project, is explicitly placed in the proper package directory for publishing. Alternatively, the metadata would be placed in an unpackaged package directory if it's never meant for actual publication as part of a package.

Either way, the default unsorted directory is the "safe landing" spot where metadata waits while its end-state location is determined.

@mshanemc, the big problem with this practice is the awful way the CLI handles reorganization. I've just made it a habit of moving metadata before refreshing a scratch org, so I don't have to deal with the CLI and the org getting out of sync. I'd love it if there were a better way. Please, Obi-Wan Kenobi, you're our only hope on this one. ;-)

[sirephil]

@mshanemc what @VivekMChawla said... 😄

[jwoodwardcertinia]

This is super important on big projects (or even medium projects). You're letting us use the file system to organise code (thank you!), and as projects grow and evolve one wants to move the code around. Devs are not moving code to where they know it should be because they are actively avoiding the deploy errors they know it will cause. Nobody wants to post to a large group of devs "I've just merged a change that's going to break your deploy, maybe you can salvage it by deploying these files separately and doing some other steps. Or maybe you'll have to get a new one and sit through a long deploy."

[mshanemc]

case 2: renaming

rename a file in place (ex: force-app/main/default/classes/Lightening.cls => force-app/main/default/classes/Lightning.cls ) [of course, you'd need to rename the class's .xml too, plus all the things that depend on it]

This does require a deploy, so that the name on the server gets fixed, too. We'd differentiate case 2 vs. case 1 because the sha is the same but the deleted file and the created file have different names.

This one is going to be trickier to detect because

1. you could have the same contents in 2 files for some reason. We'd have to try to match the correct one, hopefully the 2 matching files are not both in modified status.
2. some types of metadata also include their name (developerName, fullName, label, etc) so then matching them becomes much harder (you can't rely on the sha).

Vote this comment up if the use case is important to you.

[sirephil]

You cannot rely on detail from the version control system because not all Salesforce development necessarily uses git (some businesses use other version control systems). I don't say this on my own behalf since we do use git.

Renaming a metadata item will often cause content in many other files (and its own file) to change, especially when using refactoring tools. Assuming the content hash is the same is a non-starter.

[aheber]

This could get fun if you think about this on a per-file basis. Those .xml companion files almost always have similar or identical contents, you'll end up deferring to the primary file for things like that and somewhat ignoring a move of the xml file. I'm sure there are some edge cases that will hurt on this road.

[aheber]

It should also be noted, the "git" in question is the internal state-tracking repository used by the CLI to track changes, not the repository/tooling used by the individual and larger team. (I think)

[mshanemc]

You cannot rely on detail from the version control system because not all Salesforce development necessarily uses git

yes, @aheber is correct. We track local file changes in a separate .git folder inside .sf. You don't need to (but can) interact with it. the CLI brings its own git implementation so you don't have to have one (though most of you do).

[mshanemc]

There's also the question of "auto-detection" vs. "explicit intent". For example, git has git mv...the CLI could have a similar command so that we don't have to try to figure out what you wanted, which file, etc.

If you're moving a file to a new place but have the same basename, we know that's 1: reorganizing and if the file basename has changed, we know that's 2: renaming. And then the command can take the appropriate steps.

Feel free to comment if you think this should require a specific command vs. trying to "figure it out".

[petter-eikeland]

I would argue for the "auto-detection" behavior. This is because you are not only moving/renaming the files locally on behalf of yourself. You are also affecting everyone else contributing on the same project. The moment other people merge your changes on main onto their branch branch, they will experience the same deployment issues. Thus, it wouldn't make sense for them to show "explicit intent" for remote changes.

[sirephil]

I still point to the fact that some VCSs need specific commands to be executed to understand a rename (yes, I know git doesn't, but that's not representative of all VCSs).

[jwoodwardcertinia]

I have similar concerns to @sirephil . If you use git mv or some new command to move it, that's ok for your source tracking and your org. What's going to happen when I pull those changes? Will you be able to "take the appropriate steps" to deploy to my org?

[mshanemc]

What's going to happen when I pull those changes

that is a good point. SourceMembers will definitely think of this as, "you deleted one and created another" and the CLI would have to adjust for that. That just gave us about a dozen test cases.

[jefersonchaves]

I would love to have auto-detection like git.

[geoffswift]

I just tried moving an Apex class into a different folder.

git mv force-app/tests/utils/TestDataBuilder.cls* force-app/tests/default/classes/utils/

When I try to do a push the result is as follows:

STATE   FULL NAME               TYPE                     PROJECT PATH                                                                             
 ─────── ─────────────────────── ──────────────────────── ──────────────────────────────────────────────────────────────────────────────────────── 
 Deleted TestDataBuilder         ApexClass                force-app/tests/utils/TestDataBuilder.cls                                                
 Deleted TestDataBuilder         ApexClass                force-app/tests/utils/TestDataBuilder.cls-meta.xml                                       
 Deleted TestDataBuilder         ApexClass                force-app/tests/default/classes/utils/TestDataBuilder.cls                                
 Deleted TestDataBuilder         ApexClass                force-app/tests/default/classes/utils/TestDataBuilder.cls-meta.xml                       

Presumably it thinks that because the original file isn't there, I want to remove the class and the result is the class is undeployed. It's evidently detected there are two copies of the file and reportd both are removed. Perhaps it can sense check what's going on here, two wrongs make a right - two removes of the same API name equals a rename?

It's a real hassle for us to have to reset source tracking and do a push to correct the situation, because our code bases are large. This is a real time sync for us especially when we're talking about several large teams all being impacted simultaneously. In my opinion this is the single worst thing about the sf tooling which I implore the maintainers to fix as high priority.

[iowillhoit]

Hey all 👋 We just shipped the automatic detection of moved files last night in the nightly build (2.43.0)!
It is currently hidden behind an environment variable. To try it out, install nightly and set SF_BETA_TRACK_FILE_MOVES=true

Details: during a "get git status", we perform several small incremental steps and exit as early as we can to not affect performance. The general flow of how this works is:

• We check the local git status for both deleted AND added files
• We compare those two arrays to see if there are file basename matches
• If there are, we do a few more checks to ensure the files are the same:
  • We collect and compare the git object id (a sha-1 hash of file metadata)
  • We remove any duplicates in the adds or deletes (these will not get committed)
  • We then resolve the metadata type to ensure:
    • The type name matches
    • The parent name matches (or is undefined)
    • The parent type name matches
• If all of those checks pass, we commit the deleted and added files for you.

Let me know if you have any questions or if you come across any issues!

The pull request for the feature is here: forcedotcom/source-tracking#574

[sirephil]

@VivekMChawla ah, so it internally uses its own git repo to track local changes? Fair enough.

[ntaylorcertinia]

Gave this a test on one of our larger repos and unfortunately it didn't seem able to handle it. I installed the nightly and set the flag, then tried to push. This repo is ~8GB in size and has enough metadata in it it requires breaking the push into multiple stages to allow it to deploy normally. The push in question ran without the flag was pushing ~1700 files.

Unfortunately it hit a heap limit after 10-15 minutes

<--- Last few GCs --->

[34733:0x140008000]   569555 ms: Scavenge 3989.5 (4132.9) -> 3986.5 (4132.9) MB, 29.42 / 0.00 ms  (average mu = 0.761, current mu = 0.643) allocation failure; 
[34733:0x140008000]   569760 ms: Scavenge 3990.8 (4132.9) -> 3988.8 (4132.9) MB, 39.04 / 0.00 ms  (average mu = 0.761, current mu = 0.643) allocation failure; 
[34733:0x140008000]   570021 ms: Scavenge 3994.0 (4132.9) -> 3991.3 (4148.9) MB, 71.96 / 0.00 ms  (average mu = 0.761, current mu = 0.643) allocation failure; 


<--- JS stacktrace --->

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
----- Native stack trace -----

 1: 0x104e5d0ac node::OOMErrorHandler(char const*, v8::OOMDetails const&) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
 2: 0x104fe2ff4 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, v8::OOMDetails const&) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
 3: 0x1051b753c v8::internal::Heap::GarbageCollectionReasonToString(v8::internal::GarbageCollectionReason) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
 4: 0x1051b6018 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
 5: 0x1051ac830 v8::internal::HeapAllocator::AllocateRawWithLightRetrySlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
 6: 0x1051ad090 v8::internal::HeapAllocator::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
 7: 0x1051919c4 v8::internal::Factory::AllocateRaw(int, v8::internal::AllocationType, v8::internal::AllocationAlignment) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
 8: 0x1051889cc v8::internal::FactoryBase<v8::internal::Factory>::NewConsString(v8::internal::Handle<v8::internal::String>, v8::internal::Handle<v8::internal::String>, int, bool, v8::internal::AllocationType) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
 9: 0x105188498 v8::internal::FactoryBase<v8::internal::Factory>::NewConsString(v8::internal::Handle<v8::internal::String>, v8::internal::Handle<v8::internal::String>, v8::internal::AllocationType) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
10: 0x1050054ac v8::String::Concat(v8::Isolate*, v8::Local<v8::String>, v8::Local<v8::String>) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
11: 0x104d87428 node::UVException(v8::Isolate*, int, char const*, char const*, char const*, char const*) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
12: 0x104e62d34 node::fs::FSReqAfterScope::Reject(uv_fs_s*) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
13: 0x104e642ec node::fs::AfterScanDir(uv_fs_s*) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
14: 0x104e4ca9c node::MakeLibuvRequestCallback<uv_fs_s, void (*)(uv_fs_s*)>::Wrapper(uv_fs_s*) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
15: 0x10582a6f8 uv__work_done [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
16: 0x10582e148 uv__async_io [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
17: 0x105840220 uv__io_poll [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
18: 0x10582e70c uv_run [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
19: 0x104d816f0 node::SpinEventLoopInternal(node::Environment*) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
20: 0x104e9c918 node::NodeMainInstance::Run(node::ExitCode*, node::Environment*) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
21: 0x104e9c698 node::NodeMainInstance::Run() [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
22: 0x104e1bd1c node::Start(int, char**) [/Users/nicholastaylor/.nvm/versions/node/v20.12.0/bin/node]
23: 0x1920a20e0 start [/usr/lib/dyld]
[1]    34733 abort      sfdx project deploy start -c

So I ran the command with --max-old-space-size=8000 to give it a bit more memory and see if it could run, it ran for about an hour and then threw the same error.

After that as another test I manually moved a single labels file from one package directory to another and tried the push again, this was to see if the issue stemmed from the size of the push or the size of the repo and unfortunately it still fell over.

Final test I did here was trying it again with a smaller repo to make sure it is not my machine, this worked quite nicely so it seems that primarily it is just large repos that don't work currently. Though one piece of feedback here, the push had a noticeable delay before it started which also happens if a lot of files have changed but it just looks like it is hanging as there is no visible feedback to the user. It would be good to put a message like "Scanning changes" here to make it clear to the user something is happening.

All in all while this feature is something that would be really valuable to us it will be difficult to adopt unless it can be made to scale to handle larger repos.

[VivekMChawla]

@ntaylorcertinia - Thank you for testing this and giving us this feedback.

I'd like to create an issue from your comment, but I wanted to double-check a few things first.

1. What kind of source files were you moving? I think it's Custom Labels because you mentioned "moving a single labels file", but I wanted to ensure we're on the same page.
2. Are you using any of the new source decomposition (i.e. registry variant) options?
3. How long does the specific push (the 1700 file one) take when Source Mobility (i.e. SF_BETA_TRACK_FILE_MOVES) feature is NOT enabled?

@iowillhoit - Any other details we should get before turning this into an issue?

[ntaylorcertinia]

@VivekMChawla

1. So there are two cases mentioned above that I should separate.
   The first was a general initial test (the 1700 file push) this was a mix of files being moved by me merging develop (the main branch) into my local and then attempting to push it. This merge included moves of files of a mix of types Apex Classes, LWCs, Custom Labels and more as a bunch of files had been moved late last week making a new folder. After that failed I wanted to confirm it was indeed the new feature and not the version of the CLI in general, so I turned it off got a new org (unfortunately making recreating this specific initial 'real world' situation difficult) and then used that for other testing.
   The second scenario was that after the first failed I wanted to test if a single file being pushed would also have the same issue, so I moved a single Custom Labels file and the heap issue and slow push time also happened in that case.
2. No we are not currently using the new source decomposition. As it is an all of nothing switch over for each type of metadata it will be quite a time intensive operation to migrate such a large repo to use that so we have not started to do that yet.
3. Unfortunately I did a few things after so don't have the exact time stamps in my terminal anymore (do let me know if this is logged anywhere and I can see about getting it) but it took no longer than a normal push of that size (they are not uncommon at that size) for me so maybe 5-10 minutes.

[VivekMChawla]

@ntaylorcertinia - Thanks for the additional info. I've combined your feedback into this issue: #2880