vscode-perforce icon indicating copy to clipboard operation
vscode-perforce copied to clipboard

Moving (reopen) symlink to other changlist tries to move the symlink target

Open jhollowe opened this issue 2 years ago • 4 comments

Describe the bug:

When attempting to move ("reopen" in perforce terms) an added symlink from the default changelist to another changelist, the extension tries to move the symlink's target instead of the symlink itself.

Expected behavior:

moves the symlink without trying to evaluate it to an absolute path and thus getting the target file.

To Reproduce:

  • have file.txt checked into perforce
  • create a changelist (say 12345)
  • create a symlink to file.txt (ln -s file.txt symlink-to-file.txt)
  • add the file to perforce in the default changelist (p4 add symlink-to-file.txt)
  • refresh the extension to see the added symlink-to-file.txt
  • Use the "Move to changelist..." option of the extension and select the existing changelist (12345)
  • Note the command attempted in the logs is p4 'reopen' '-c' '12345' 'file.txt' which fails with file(s) not opened on this client.

Versions & Details:

  • Perforce extension: v4.15.5
  • VSCode version: 1.73.1
  • Operating System: Windows 10 frontend SSH-remoted into RHEL7.9
  • Perforce server details (if known): P4D/LINUX26X86_64/2019.2/1927395 (2020/03/04)
  • Are you using a multi-root workspace?: Yes/No

Additional context

Please open view -> Output, select "Perforce Log" from the dropdown and paste it here, if possible / relevant

jhollowe avatar Dec 05 '22 19:12 jhollowe

Hi, thanks for reporting. I'm running on macos right now and I couldn't reproduce the problem - when I right click on the symlink file and move to a changelist it correctly moves the file and uses the name of the symlink (e.g. symlink-to-file.txt) in the command.

In terms of implementation:

  • when you run refresh, it uses p4 opened to find all the open files and then p4 fstat on each depot path.
  • From fstat it uses the 'clientFile' field on the to get the file name.
  • We then feed this file name into the reopen command

so, if you run p4 fstat on the depot path, which file name does it return?

mjcrouch avatar Dec 08 '22 17:12 mjcrouch

jhollowe@dev:/nfs/p4_clones/testing$ p4 fstat //depot/dev/README
... depotFile //depot/dev/README
... clientFile /nfs/p4_clones/testing/README
... isMapped 
... headAction edit
... headType text
... headTime 1669128188
... headRev 17
... headChange 6526010
... headModTime 1669125848
... haveRev 17

jhollowe@dev:/nfs/p4_clones/testing$ ln -s README README.symlink
jhollowe@dev:/nfs/p4_clones/testing$ ls -halF README*
-r--r--r-- 1 jhollowe users 6.9K Nov 22 09:57 README
lrwxrwxrwx 1 jhollowe users    6 Jan  3 11:57 README.symlink -> README

jhollowe@dev:/nfs/p4_clones/testing$ p4 fstat README.symlink 
README.symlink - no such file(s).
jhollowe@dev:/nfs/p4_clones/testing$ p4 add README.symlink
//depot/dev/README.symlink#1 - opened for add
jhollowe@dev:/nfs/p4_clones/testing$ p4 client
Change 6560427 created with 1 open file(s).
jhollowe@dev:/nfs/p4_clones/testing$ p4 fstat README.symlink 
... depotFile //depot//dev/README.symlink
... clientFile /nfs/p4_clones/testing/README.symlink
... isMapped 
... action add
... change 6560427
... type symlink
... actionOwner jhollowe
... workRev 1

jhollowe@dev:/nfs/p4_clones/testing$ 

When I open README.symlink in the main editor, the Perforce Log contains the following:

/nfs/p4_clones/testing: p4 'have' '/nfs/p4_clones/testing/README'
Detected conflicting status for file file:///nfs/p4_clones/testing/README.symlink
SCM provider believes the file is open, but latest 'opened' call does not.
This is probably caused by an external change such as submitting or reverting the file from another application.
/nfs/p4_clones/testing: p4 'print' '-q' '/nfs/p4_clones/testing/README#have'

Why I try to move the symlink from the changelist I created to the default changelist, the following error happens:

/nfs/p4_clones/testing: p4 'reopen' '-c' 'default' '/nfs/p4_clones/testing/README'
ERROR: "/nfs/p4_clones/testing/README - file(s) not opened on this client.\n"
ERROR: "/nfs/p4_clones/testing/README - file(s) not opened on this client.\n"
ERROR: "/nfs/p4_clones/testing/README - file(s) not opened on this client.\n"

jhollowe avatar Jan 03 '23 18:01 jhollowe

After looking through my settings to make sure there was nothing broken, I have "perforce.realpath": true, set. When setting this to false, the issue no longer is present.

While it makes sense why this is failing, I would expect the perforce.realpath functionality to only get the absolute path of the workspace and not try to convert symlinks within the workspace root to be an absolute path.

jhollowe avatar Jan 03 '23 18:01 jhollowe

thanks for following up - realpath was implemented before I forked the extension and I'd forgotten it existed. It looks like the use case for this feature was for someone who had symlinked individual files from a perforce depot into a different folder and wanted to make operations on those files, even though they didn't have the p4 workspace open ( https://github.com/stef-levesque/vscode-perforce/issues/42 ) - a bit different from your use case for just resolving the top-level workspace.

mjcrouch avatar Jan 03 '23 18:01 mjcrouch