Skip to content

Skip cloud files marked as not on disk during command discovery #18152

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.

Sign up for GitHub

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

Merged
daxian-dbw merged 4 commits into from
Nov 8, 2022
Merged

Skip cloud files marked as not on disk during command discovery #18152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a5b291
Skip files marked as offline or not fully on disk
SeeminglyScience Sep 23, 2022
b9081fe
Wrap changes in an experimental feature flag
SeeminglyScience Sep 27, 2022
7208e04
Merge branch 'master' into skip-cloud-command-discovery
SeeminglyScience Oct 17, 2022
2f83e04
Merge branch 'master' into skip-cloud-command-discovery
SeeminglyScience Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions experimental-feature-linux.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
"PSLoadAssemblyFromNativeCode",
"PSNativeCommandErrorActionPreference",
"PSSubsystemPluginModel",
"PSModuleAutoLoadSkipOfflineFiles",
"PSFeedbackProvider"
]
1 change: 1 addition & 0 deletions experimental-feature-windows.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
"PSLoadAssemblyFromNativeCode",
"PSNativeCommandErrorActionPreference",
"PSSubsystemPluginModel",
"PSModuleAutoLoadSkipOfflineFiles",
"PSFeedbackProvider"
]
4 changes: 4 additions & 0 deletions src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class ExperimentalFeature

internal const string EngineSource = "PSEngine";
internal const string PSNativeCommandErrorActionPreferenceFeatureName = "PSNativeCommandErrorActionPreference";
internal const string PSModuleAutoLoadSkipOfflineFilesFeatureName = "PSModuleAutoLoadSkipOfflineFiles";
internal const string PSCustomTableHeaderLabelDecoration = "PSCustomTableHeaderLabelDecoration";
internal const string PSFeedbackProvider = "PSFeedbackProvider";

Expand Down Expand Up @@ -118,6 +119,9 @@ static ExperimentalFeature()
new ExperimentalFeature(
name: PSNativeCommandErrorActionPreferenceFeatureName,
description: "Native commands with non-zero exit codes issue errors according to $ErrorActionPreference when $PSNativeCommandUseErrorActionPreference is $true"),
new ExperimentalFeature(
name: PSModuleAutoLoadSkipOfflineFilesFeatureName,
description: "Module discovery will skip over files that are marked by cloud providers as not fully on disk."),
new ExperimentalFeature(
name: PSCustomTableHeaderLabelDecoration,
description: "Formatting differentiation for table header labels that aren't property members"),
Expand Down
57 changes: 55 additions & 2 deletions src/System.Management.Automation/engine/Modules/ModuleUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,46 @@ namespace System.Management.Automation.Internal
{
internal static class ModuleUtils
{
// These are documented members FILE_ATTRIBUTE, they just have not yet been
// added to System.IO.FileAttributes yet.
private const int FILE_ATTRIBUTE_RECALL_ON_DATA_ACCESS = 0x400000;

private const int FILE_ATTRIBUTE_RECALL_ON_OPEN = 0x40000;
Copy link
Collaborator Author

@SeeminglyScience SeeminglyScience Sep 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not actually been able to produce a scenario where this attribute appears (despite checking from multiple win32 APIs in case it only appeared in certain structs). I kept it in incase there's a different cloud provider that uses it, or if OneDrive starts to use it at some point.

Documentation for FILE_ATTRIBUTE_RECALL_ON_OPEN reads:

This attribute only appears in directory enumeration classes (FILE_DIRECTORY_INFORMATION, FILE_BOTH_DIR_INFORMATION, etc.). When this attribute is set, it means that the file or directory has no physical representation on the local system; the item is virtual. Opening the item will be more expensive than normal, e.g. it will cause at least some of it to be fetched from a remote store.

Copy link
Collaborator

@iSazonov iSazonov Sep 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can't even check it manually and even OneDrive doesn't use it, why is it PR?

Copy link
Collaborator Author

@SeeminglyScience SeeminglyScience Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept it in incase there's a different cloud provider that uses it, or if OneDrive starts to use it at some point. Basically, documentation tell us to look for it, so I lean towards looking for it. Even if it happens not to be present in the specific use case we're looking to solve.

Copy link
Collaborator

@iSazonov iSazonov Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You won't be the first anyway. 😸
https://github.com/search?q=org%3Adotnet+FILE_ATTRIBUTE_RECALL_ON_OPEN&type=issues


// Default option for local file system enumeration:
// - Ignore files/directories when access is denied;
// - Search top directory only.
private static readonly System.IO.EnumerationOptions s_defaultEnumerationOptions =
new System.IO.EnumerationOptions() { AttributesToSkip = FileAttributes.Hidden };
new System.IO.EnumerationOptions() { AttributesToSkip = FileAttributesToSkip };

private static readonly FileAttributes FileAttributesToSkip;

// Default option for UNC path enumeration. Same as above plus a large buffer size.
// For network shares, a large buffer may result in better performance as more results can be batched over the wire.
// The buffer size 16K is recommended in the comment of the 'BufferSize' property:
// "A "large" buffer, for example, would be 16K. Typical is 4K."
private static readonly System.IO.EnumerationOptions s_uncPathEnumerationOptions =
new System.IO.EnumerationOptions() { AttributesToSkip = FileAttributes.Hidden, BufferSize = 16384 };
new System.IO.EnumerationOptions() { AttributesToSkip = FileAttributesToSkip, BufferSize = 16384 };

private static readonly string EnCulturePath = Path.DirectorySeparatorChar + "en";
private static readonly string EnUsCulturePath = Path.DirectorySeparatorChar + "en-us";

static ModuleUtils()
{
if (ExperimentalFeature.IsEnabled(ExperimentalFeature.PSModuleAutoLoadSkipOfflineFilesFeatureName))
{
FileAttributesToSkip = FileAttributes.Hidden
// Skip OneDrive files/directories that are not fully on disk.
| FileAttributes.Offline
| (FileAttributes)FILE_ATTRIBUTE_RECALL_ON_DATA_ACCESS
| (FileAttributes)FILE_ATTRIBUTE_RECALL_ON_OPEN;

return;
}

FileAttributesToSkip = FileAttributes.Hidden;
}

/// <summary>
/// Check if a directory is likely a localized resources folder.
/// </summary>
Expand Down Expand Up @@ -276,6 +300,11 @@ internal static IEnumerable<string> GetDefaultAvailableModuleFiles(string topDir
manifestPath += StringLiterals.PowerShellDataFileExtension;
if (File.Exists(manifestPath))
{
if (HasSkippedFileAttribute(manifestPath))
{
continue;
}

isModuleDirectory = true;
yield return manifestPath;
}
Expand All @@ -288,6 +317,11 @@ internal static IEnumerable<string> GetDefaultAvailableModuleFiles(string topDir
string moduleFile = Path.Combine(directoryToCheck, proposedModuleName) + ext;
if (File.Exists(moduleFile))
{
if (HasSkippedFileAttribute(moduleFile))
{
continue;
}

isModuleDirectory = true;
yield return moduleFile;

Expand Down Expand Up @@ -337,6 +371,25 @@ internal static List<Version> GetModuleVersionSubfolders(string moduleBase)
return versionFolders;
}

private static bool HasSkippedFileAttribute(string path)
{
try
{
FileAttributes attributes = File.GetAttributes(path);
if ((attributes & FileAttributesToSkip) is not 0)
{
return true;
}
}
catch
{
// Ignore failures so that we keep the current behavior of failing
// later in the search.
}

return false;
}

private static void ProcessPossibleVersionSubdirectories(IEnumerable<string> subdirectories, List<Version> versionFolders)
{
foreach (string subdir in subdirectories)
Expand Down