Tree pane updates when Junctions are created

[schinagl]

Refresh left pane aka tree pane when junctions are created, but don't show them, if one has deselected junction appearance under View/File Type

[malxau-msft]

Sorry for the delay. I've been looking into this, and the symlink one, and both still have the issue where if the user is hiding hidden and system files, links get included in the left pane.

Looking further into this, part of the problem is WFMoveCopyDriverThread calls WFSymbolicLink or WFJunction, then calls WFSetAttr to apply attributes from the destination to the link. This is after ChangeFileSystem is called, so it doesn't have the opportunity to perform any hiding. The WFSetAttr behavior seemed a little strange to me, since it's not what the API will do; but since it was code you added when adding symlink/junction support, I assume this is something you want to keep.

As I see it, that means either moving the WFSetAttr into WFSymbolicLink/WFJunction, or moving the ChangeFileSystem out of those and into WFMoveCopyDriverThread after it calls WFSetAttr. Given that there are other similar functions that already call ChangeFileSystem, I'm assuming it's more consistent to move the call to WFSetAttr.

From there, InsertDirectory can check for ATTR_HS. Unfortunately it also needs to NULL the output pointer (otherwise it will return freed memory) and the callers don't seem very diligent about checking for failure. ScanDirLevel happens to be implementing the same checks as InsertDirectory, so if they're in perfect sync it won't touch the output pointer, but I think ReadDirLevel still can. At least, I watched it crash accessing a freed pointer before setting it to NULL.

One odd quirk I found while looking into this is the root directory on NTFS has the hidden/system attributes set. Previously this didn't matter since it wouldn't be returned from file enumeration so would never be hidden, but it's important to not hide it in InsertDirectory either. If it is hidden, the tree will have zero items, causing the program to enter an infinite loop trying to populate the tree forever (where InitFileManager calls TC_SETDRIVE.)

The changes I've been working with are at https://github.com/malxau-msft/winfile/commit/485bd11cb96681290780f893694b5055afb7038d . I'm not attached to anything here and am not very confident in the error handling from InsertDirectory, although it appears to work for all of the cases I've tried so far.

[schinagl]

Sorry for the delay. I've been looking into this, and the symlink one, and both still have the issue where if the user is hiding hidden and system files, links get included in the left pane.

You are mixing up things I gess. See my comment with #406.

Looking further into this, part of the problem is WFMoveCopyDriverThread calls WFSymbolicLink or WFJunction, then calls WFSetAttr to apply attributes from the destination to the link. This is after ChangeFileSystem is called, so it doesn't have the opportunity to perform any hiding. The WFSetAttr behavior seemed a little strange to me, since it's not what the API will do; but since it was code you added when adding symlink/junction support, I assume this is something you want to keep.

As I see it, that means either moving the WFSetAttr into WFSymbolicLink/WFJunction, or moving the ChangeFileSystem out of those and into WFMoveCopyDriverThread after it calls WFSetAttr. Given that there are other similar functions that already call ChangeFileSystem, I'm assuming it's more consistent to move the call to WFSetAttr.

Completley disagree! We must not mix up changing creation of symbolic links and changing attributes. This is bad design and no other function in Winfile does this.

From there, InsertDirectory can check for ATTR_HS. Unfortunately it also needs to NULL the output pointer (otherwise it will return freed memory) and the callers don't seem very diligent about checking for failure. ScanDirLevel happens to be implementing the same checks as InsertDirectory, so if they're in perfect sync it won't touch the output pointer, but I think ReadDirLevel still can. At least, I watched it crash accessing a freed pointer before setting it to NULL.

I only agree with measures on setting ppNode to nullptr as shown in your snippet, but a check for `ATTR_HS seems not related to this PR. Why woould you want to change ReadDirLevel()? Which problem are you addressing?

One odd quirk I found while looking into this is the root directory on NTFS has the hidden/system attributes set. Previously this didn't matter since it wouldn't be returned from file enumeration so would never be hidden, but it's important to not hide it in InsertDirectory either. If it is hidden, the tree will have zero items, causing the program to enter an infinite loop trying to populate the tree forever (where InitFileManager calls TC_SETDRIVE.)

This was never a problem before, why should it be a problem now? Please describe a use-case of failure.

Summary Can you please tell me the problem with my solution. (except ppNode = nullptr, which I am about to add. Thx! ) To me it seems you are describing other problems not related to this PR with ATTR_HS & co

So please describe a use-case where my solutions fails.

[malxau-msft]

So please describe a use-case where my solutions fails.

1. Under View Options, ensure "Show Junction Points" is set, and "Show Hidden/System Files" is not set.
2. Create a directory.
3. Mark that directory hidden.
4. Create a junction that points to that directory.
5. Restart winfile.

Without this PR:

1. The newly created junction will not be displayed in the left pane or the right pane.
2. On restart, the junction will not be displayed in the left pane or the right pane.

With this PR:

1. The newly created junction will be displayed in the left pane but not the right pane.
2. On restart, the junction will not be displayed in the left pane or the right pane.

Normally, Winfile applies ATTR_HS filtration in WFFindFirst and WFFindNext. This happens before the reparse tag is inspected, and applies to symbolic links in the same way as junctions. Calling InsertDirectory directly bypasses this filtration, so this PR is populating the left pane with things that it normally wouldn't contain.

Completley disagree! We must not mix up changing creation of symbolic links and changing attributes. This is bad design and no other function in Winfile does this.

Feel free to propose an alternative solution. Looking today, another valid approach would be to ensure WFSetAttr refreshes the left pane (it's already calling ChangeFileSystem.)

[schinagl]

Thx for the repro description. Will dig into during this week

[schinagl]

Runing your experiment

With this PR: The newly created junction will be displayed in the left pane but not the right pane.

It is shown on the left and right pane, as can be see from my screenshot video https://github.com/microsoft/winfile/assets/7418603/cea43c98-98d2-46ce-b6ff-77f97ee7675f

BTW: On the right pane is it shown with or without this PR. so 1) of 'With out this PR' from above is not correct!

And after quit and restart it looks like

But this obvious because you made sure '"Show Hidden/System Files" is not set.' You can't see hidden dirs and thus you can't see Junctions inside. BTW there is no difference in your statement 2) with or without this very PR

So what???? This is correct behaviour. This is why I set up this PR:

  **After creating a junction or symlink it is not shown in the left pane**

In general showing junctions shall be completley unrelated to ATTR_HS. There is no need for an alternative solution or your proposal.

PS: But if you think the problem is, that junctions are shown in a freshly hidden directory, then this is a different story and the behaviour is the same with or without this PR. This problem is, that a freshly hidden directory does not vanish on the spot when you set it to hidden. But even this was ever since with and without this PR

[malxau-msft]

This is the behavior I am seeing:

https://github.com/microsoft/winfile/assets/104935627/e79d3378-5297-4c2a-ad17-d1658f928182

This problem is, that a freshly hidden directory does not vanish on the spot when you set it to hidden

Yes, I agree. The issue is that Winfile will set these junctions to hidden at the time they are created (at https://github.com/microsoft/winfile/blob/7ad683989e178a8fe6f5f9abfecb9821d0b56958/src/wfcopy.c#L2822), so it creates this condition in a way that is hidden from the user. That's why I'm suggesting WFSetAttr would be one way to fix it - it fixes the general case, which also fixes the case of a freshly created junction to a hidden directory.

[schinagl]

My usecase up till now was: create a junction inside a hidden directory. This works correctly https://github.com/microsoft/winfile/assets/7418603/916d8bd1-57a1-4d18-9b9f-06d1ce8bd08a ok. So far so good

Your use-case was Create a junction to a freshly hidden directory, After playing around a long time I can reproduce this.

But to be honest this is a super rare hair in the soup scenario

• One has to hide a directory
• Then live from the problem, that such a directory does not vanish on the spot ( if it would, one couldn't create a junction)
• Create a junction from a directory which shouldn't be there
• Do not collapse/expand the dir (because then it would beall correct)

This is 0.01%% scenario. In all other scnanrios it helps to see the left pane updated.

Anyhow: I would never ever introduce your proposed hack, because it mixes creation of Junctions and setting of attributes and introduces and furthermore brings in changes in many places and causes risk. We must not do this

And: I am not going to search for a solution on this. Will abandon this PR and the others I have, because it takes me way more time to discuss things than doing it.

[schinagl]

I am leaving this Winfile Repo, no need to merge