Skip to content
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.

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

refactor(unix): Remove unix.ps1 #5235

Merged
merged 4 commits into from
Nov 10, 2022
Merged

refactor(unix): Remove unix.ps1 #5235

merged 4 commits into from
Nov 10, 2022

Conversation

niheaven
Copy link
Member

@niheaven niheaven commented Nov 1, 2022

Description

Uniform paths on Windows and Linux/macOS, and remove unused unix.ps1.

Scoop's default folder is under UserProfile (C:\Users\<user> on Windows, /home/<user> on Linux/macOS), and global folder is under CommonApplicationData (C:\ProgramData on Windows, /usr/share on Linux/macOS).

Summaries:

User Root (UserProfile) User Root (LocalApplicationData) Global Root
Windows C:\Users\<user>\scoop C:\Users\<user>\AppData\Local\scoop C:\ProgramData\scoop
Linux/macOS /home/<user>/scoop /home/<user>/.local/share/scoop /usr/share/scoop

RFC

As said in https://learn.microsoft.com/en-us/dotnet/api/system.environment.specialfolder?view=netframework-4.5, UserProfile shouldn't be used to store apps and/or data.

But scoop has changed its root folder from LocalApplicationData to UserProfile in fc54b2f to fix #737, there may not be sufficient reason to change back.

Motivation and Context

Simplify code base and development on Linux/macOS platform.

How Has This Been Tested?

There aren't tests for these config.

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

@chawyehsu
Copy link
Member

chawyehsu commented Nov 1, 2022

C:\Users<user>\AppData\Local

Not a fan of this, we changed it to ~/scoop and resolved the long path issue in 2016. No reason to revert it back to break things again.

edit:
fc54b2f

@niheaven
Copy link
Member Author

niheaven commented Nov 1, 2022

Then change the special folder to UserProfile and anything else is the same.

PS, does the long path issue still existed? And is it the same one as #4327?

@niheaven
Copy link
Member Author

niheaven commented Nov 1, 2022

I've updated summaries table, have all locations. So keep it as is? @ScoopInstaller/maintainers

@rashil2000
Copy link
Member

C:\Users<user>\AppData\Local

I'm in favor of this location as well (I don't like polluting my user home folder, and besides, LocalAppData is the recommended location for installing apps), but this is a HUGE breaking change, so I'm not sure if we should do this.

We should capitalize the S in scoop. Like C:\Users\<user>\AppData\Local\Scoop it looks more professional this way.

Also, we need to change backslashes to forward slashes, otherwise Scoop will remain incompatible on Unix.

@rashil2000
Copy link
Member

Also @niheaven, did you update the branch protection rules for buckets? The excavator action is failing in Main bucket.

@niheaven
Copy link
Member Author

niheaven commented Nov 1, 2022

Also @niheaven, did you update the branch protection rules for buckets? The excavator action is failing in Main bucket.

Yes,I forgot excavator's permission😅 Now protection rules are disabled.

@niheaven
Copy link
Member Author

niheaven commented Nov 1, 2022

Also, we need to change backslashes to forward slashes, otherwise Scoop will remain incompatible on Unix.

PowerShell uses backslashes even on Unix, and can coerce them easily. The only exception is .NET, which only recognizes slashes on Unix. But a good news here is .NET could use slashes on Windows too.

Please refer to Pester 5 test files (Import-Bucket-Tests and Manifest.Tests), they are both passed on Ubuntu and macOS.

After this PR, Unix supports is ready again, at least for developers. Install Scoop normally, or git clone the repo and set $env:SCOOP_HOME/$env:SCOOP_CACHE to correct location, then all "bin" files should by used, such as downloading and update version.

@chawyehsu
Copy link
Member

I'm in favor of this location as well (I don't like polluting my user home folder, and besides, LocalAppData is the recommended location for installing apps)

it looks more professional this way.

If this is what you believe in a professional way, then you should not use Scoop to manage apps. Scoop does not always follow Windows conventions in design, it provides a *nix user interface while its internal implementation evolves. The codebase still uses the architecture of /bin, /lib, and /libexec.

If this is what you believe in a professional way, I assume that you want to challenge the core philosophy of Scoop. Scoop puts apps' data to a persist folder rather than the ~\AppData\Romaing. Lots of the manifests are crafted carefully by contributors to persist data to the persist folder. Does this look unprofessional to you? Do you want to break this in a "professional way", in other words, the Windows conventional way you may say? Then what about the cache, should it go to $env:TEMP since that will be a professional way? If so, you're actually challenging the philosophy.

C:\Users\<user>\AppData\Local\scoop has been practiced and it's terrible because of the long path hell on Windows. And the problem is still there despite the LongPathsEnabled option. Ignoring the problem and changing to use it thinking it's a professional way is in fact not a professional way to me.

@niheaven
Copy link
Member Author

niheaven commented Nov 2, 2022

Hmm, I think @rashil2000's professional way is Scoop, the first capital letter?

I've changed back to UserProfile and now except for Unix Global Path, all is the same as before.

@rashil2000
Copy link
Member

rashil2000 commented Nov 2, 2022

Jeez @chawyehsu, I was talking about the capital 'S' when I said professional. I'm actually against the location change (see my comment above).

@chawyehsu
Copy link
Member

I'm sorry for the misunderstanding. But I still stand for the point I told: there shouldn't be a judgment of what's more professional in this case. Why would S be more professional than another that has been used since day one... S may be more conventional or classical, but s is definitely not so-called less professional.

Further, I would use the phrase Like C:\Users\<user>\Scoop... to describe the case for less ambiguity.

@niheaven niheaven requested a review from chawyehsu November 8, 2022 08:14
@niheaven
Copy link
Member Author

I'll merge it since nobody would review it 😥

rashil2000
rashil2000 previously approved these changes Nov 10, 2022
@chawyehsu
Copy link
Member

I would leave scoop as-is rather than Scoop since you want it to be used on *nix based on the purpose of this PR, and there is no point in changing it to a so-called more professional capitalized one. Because it is not tied to the Windows platform.

All the other changes are fine.

@niheaven
Copy link
Member Author

That's right, on Linux/macOS lower capital such as scoop is more common since it's capital sensitive.

So change back to scoop and that's all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants