Add set permission for certain packages #17299
Conversation
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
| .DefaultIfEmpty(default) | ||
| .MaxBy(r => r.package.Identity.Version); | ||
| var stableVersions = accumulativeSearchResults | ||
| .Where(r => !r.package.Identity.Version.IsPrerelease); |
There was a problem hiding this comment.
This is a bug just discovered when accumulativeSearchResults is empty
wli3
force-pushed
the
add-set-permission
branch
from
d5d3c4a to
15adf88
Compare
|
|
||
| private static bool PackageIsInAllowList(IEnumerable<string> files) | ||
| { | ||
| var allowListOfPackage = new string[] {"microsoft.android.sdk.darwin"}; |
There was a problem hiding this comment.
@rolfbjarne can confirm tomorrow, but it looks like Microsoft.iOS.Sdk also has a tools folder where any file without an extension would need execute permission.
Rolf, are there any other files you know of?
There was a problem hiding this comment.
These are our executable files:
Microsoft.MacCatalyst.Sdk:
-rwxr-xr-x 1 rolf wheel 155 Jan 15 10:08:34 2021 Microsoft.MacCatalyst.Sdk/tools/bin/bgen*
-rwxr-xr-x 1 rolf wheel 165 Apr 29 10:41:28 2021 Microsoft.MacCatalyst.Sdk/tools/bin/mlaunch*
-rwxr-xr-x 1 rolf wheel 149 Apr 29 10:36:13 2021 Microsoft.MacCatalyst.Sdk/tools/lib/bgen/bgen*
Microsoft.iOS.Sdk:
-rwxr-xr-x 1 rolf wheel 155 Jan 15 10:08:34 2021 Microsoft.iOS.Sdk/tools/bin/bgen*
-rwxr-xr-x 1 rolf wheel 165 Apr 29 10:41:28 2021 Microsoft.iOS.Sdk/tools/bin/mlaunch*
-rwxr-xr-x 1 rolf wheel 149 Apr 29 10:36:13 2021 Microsoft.iOS.Sdk/tools/lib/bgen/bgen*
-rwxr-xr-x 1 rolf wheel 6037104 Apr 29 10:41:27 2021 Microsoft.iOS.Sdk/tools/lib/mlaunch/mlaunch.app/Contents/MacOS/mlaunch*
Microsoft.macOS.Sdk:
-rwxr-xr-x 1 rolf wheel 155 Jan 15 10:08:34 2021 Microsoft.macOS.Sdk/tools/bin/bgen*
-rwxr-xr-x 1 rolf wheel 149 Apr 29 10:36:13 2021 Microsoft.macOS.Sdk/tools/lib/bgen/bgen*
Microsoft.tvOS.Sdk:
-rwxr-xr-x 1 rolf wheel 155 Jan 15 10:08:34 2021 Microsoft.tvOS.Sdk/tools/bin/bgen*
-rwxr-xr-x 1 rolf wheel 165 Apr 29 10:41:28 2021 Microsoft.tvOS.Sdk/tools/bin/mlaunch*
-rwxr-xr-x 1 rolf wheel 149 Apr 29 10:36:13 2021 Microsoft.tvOS.Sdk/tools/lib/bgen/bgen*
-rwxr-xr-x 1 rolf wheel 6037104 Apr 29 10:41:27 2021 Microsoft.tvOS.Sdk/tools/lib/mlaunch/mlaunch.app/Contents/MacOS/mlaunch*
There was a problem hiding this comment.
So I'll add Microsoft.MacCatalyst.Sdk, Microsoft.iOS.Sdk, Microsoft.macOS.Sdk, Microsoft.tvOS.Sdk
and just mark everything without an extension under tools executable
| internal IEnumerable<FilePath> FindAllFilesNeedExecutablePermission(IEnumerable<string> files, | ||
| string targetPath) | ||
| { | ||
| if (!PackageIsInAllowList(files)) |
There was a problem hiding this comment.
Is this a permanent solution? It seems like the allow list will get quickly out of hand when there are more workloads, are we planning on implementing a way for workloads to provide their own allow list?
There was a problem hiding this comment.
I think the permanent plan is #16894.
If workload packs could define data/UnixFilePermissions.xml, we could remove this. 👍
dotnet#11860. NuGet doesn't support preserving the executable bit for files in *.nupkgs (which are just zip files), so .NET implemented a [temporary workaround](dotnet/sdk#17299) for workload installs, where they hardcoded files that should be executable. This isn't really sustainable, and they'll remove their workaround, so we need to use their supported method of specifying the file permissions: a data/UnixFilePermissions.xml file. Fixes dotnet#11860.
#11860. (#11869) NuGet doesn't support preserving the executable bit for files in *.nupkgs (which are just zip files), so .NET implemented a [temporary workaround](dotnet/sdk#17299) for workload installs, where they hardcoded files that should be executable. This isn't really sustainable, and they'll remove their workaround, so we need to use their supported method of specifying the file permissions: a data/UnixFilePermissions.xml file. Fixes #11860.
Context: dotnet/sdk#16894 Context: dotnet/sdk#17299 On non-Windows platforms, the `dotnet workload install` command needs to ensure that certain files are executable after extracting them from the workload package, via `chmod 755 FILE`. This is currently done for all files in the `tools` folder for [select workloads][0]. dotnet/sdk/#16894 proposes using a new `data/UnixFilePermissions.xml` file to explicitly specify which files within the workload should have which Unix file permissions applied: <FileList> <File Path="tools/Darwin/aapt2" Permission="755" /> </FileList> Update `Microsoft.Android.Sdk.proj` to create a `data/UnixFilePermissions.xml` file, when building .NET 6+ installers on Linux and macOS platforms. `UnixFilePermissions.xml` is generated with the new `<GenerateUnixFilePermissions/>` task, building upon the `@(AndroidSdkBuildTools)` & related item groups in `create-installers.targets`. This is being done *before* `dotnet workload install` is updated to use `data/UnixFilePermissions.xml`. We also found that the following files should be *removed* from our .NET workload packs: * `aprofutil` * `mono` * `mono-symbolicate` `aprofutil` is needed for AOT use, and AOT is not currently supported under .NET 6. `mono` was only needed for `jnimarshalmethod-gen.exe`, which doesn't work on .NET 6, and `mono-symbolicate` *should* come from Mono-related workload packages, not Android-related packages, and also doesn't work. [0]: https://github.com/dotnet/sdk/blob/17c734cde97c6dde9d4bf298e7a2a7a342263ce6/src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs#L186-L191
No description provided.