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

Count installed Scoop packages if available #107

Merged
merged 1 commit into from
Dec 26, 2021

Conversation

FantasyTeddy
Copy link
Contributor

This change adds support for the Scoop installer on Windows.

It works by querying the scoop export command that lists all the installed packages and we can count the number of lines.

@grtcdr
Copy link
Member

grtcdr commented Dec 23, 2021

Lovely stuff... but I'm sure there's a directory whose entries we can count, right?

@FantasyTeddy
Copy link
Contributor Author

Well... yes I think we could, but I don't know how to determine the correct path to Scoop itself. The default path is under C:\Users\<user>\scoop but it is possible to change this path during installation and if this is the case I'm not sure how to find the correct path...

@grtcdr
Copy link
Member

grtcdr commented Dec 23, 2021

Okay, how are the benchmarks looking?

@FantasyTeddy
Copy link
Contributor Author

I tried to run the following benchmarks on my machine:

Note Average Min Max
Before this PR 61.8 ms 56.8 ms 114.6 ms
PR without Scoop installed 69.3 ms 57.8 ms 166.8 ms
PR with Scoop installed 65.5 ms 57.4 ms 133.9 ms

@grtcdr
Copy link
Member

grtcdr commented Dec 23, 2021

Thanks for the detail stats.

I can't help but notice that something is very off with these numbers, last time (a little too long ago) we benchmarked macchina, it didn't even exceed 10ms on Windows, but now it's hitting 60ms?...

Are you benchmarking cargo run or plain macchina (the binary)? Because, the former is VERY slow.

@FantasyTeddy
Copy link
Contributor Author

FantasyTeddy commented Dec 23, 2021

Oh no, I just realized a mistake I made: even though I remembered to build macchina in release mode, I ran the following command to time it:

Measure-Command { .\target\debug\macchina.exe }

Can you spot my mistake?


I ran the tests again with the correct binary and now the results look very bad for my PR:

Note Average Min Max
Before this PR 45.5 ms 40.9 ms 57.9 ms
PR without Scoop installed 601.6 ms 525.4 ms 747.0 ms
PR with Scoop installed 920.1 ms 744.5 ms 1178.7 ms

I totally agree that this is not acceptable and I will have to find a way to determine the installation path of Scoop and do the directory enumeration.

@FantasyTeddy
Copy link
Contributor Author

I tried to do it that way now: I'm looking for scoop with the which crate and find the correct path to the installed apps relative to this location.

The numbers look much better now, however there is still some impact (especially if the command for scoop cannot be found).

Note Average Min Max
Before this PR 45.5 ms 40.9 ms 57.9 ms
PR without Scoop installed 68.4 ms 51.6 ms 174.4 ms
PR with Scoop installed 53.4 ms 49.5 ms 82.9 ms

@FantasyTeddy FantasyTeddy marked this pull request as ready for review December 23, 2021 18:37
@grtcdr
Copy link
Member

grtcdr commented Dec 23, 2021

Can you spot my mistake?

Measure-Command { .\target*debug*\macchina.exe }

Yep.

The tests still show higher numbers than what is expected, can you please try benchmarking but with hyperfine instead? That's what we use for all our benchmarks.

If the scoop command can't be found, then it's safe to assume that no such tool exists, as anyone using scoop will most definitely add it to their PATH.

@FantasyTeddy
Copy link
Contributor Author

I measured it again with hyperfine and got these results:

Note Mean [ms] Min [ms] Max [ms] Relative
Latest release (6.0.1) 47.4 ± 3.1 42.7 62.6 1.00
Latest main 43.3 ± 1.1 41.0 46.4 1.00
PR without Scoop installed 54.0 ± 0.8 52.6 55.7 1.00
PR with Scoop installed 52.3 ± 1.7 49.5 60.2 1.00

I'm running an Intel® Core™ i7-4770 CPU @ 3.40GHz so it's not the fastest machine.

Cargo.toml Outdated Show resolved Hide resolved
Copy link
Member

@grtcdr grtcdr left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for adding this ❤️

@grtcdr grtcdr merged commit b84a4f3 into Macchina-CLI:main Dec 26, 2021
@FantasyTeddy FantasyTeddy deleted the count-scoop-packages branch December 26, 2021 19:03
@rashil2000
Copy link

rashil2000 commented Dec 29, 2021

Sorry for bumping a merged thread, but a few days ago I wanted to add Scoop readout to macchina but couldn't get to it. Glad to see it's landed :)

I'm looking for scoop with the which crate and find the correct path to the installed apps relative to this location.

How much cost does this dependency incur? I think we don't need it.

Well... yes I think we could, but I don't know how to determine the correct path to Scoop itself. The default path is under C:\Users\<user>\scoop but it is possible to change this path during installation and if this is the case I'm not sure how to find the correct path...

The environment variable SCOOP is what you're looking for. See https://github.com/ScoopInstaller/Scoop#install-scoop-to-a-custom-directory-by-changing-scoop

If this variable is unset, Scoop defaults to C:\Users\<user>\scoop.

So if we can check this variable, we can slightly speed things up (I'm assuming an env var lookup is cheaper than searching for an executable in the PATH using a which function).

@grtcdr
Copy link
Member

grtcdr commented Dec 29, 2021

Thank you so much @rashil2000 for this valuable info ❤️

I'm assuming an env var lookup is cheaper than searching for an executable in the PATH using a which function

Absolutely, which (no pun intended) is why LinuxPackageReadout no longer makes more calls than I can count to our own extra::which. I benchmarked the extra::which method and realized it cost around 4300ns for every call. A call to the which::which I would assume is just as slow. So, if we can avoid it, we really should.

@FantasyTeddy care to rewrite it?

@FantasyTeddy
Copy link
Contributor Author

The environment variable SCOOP is what you're looking for. See https://github.com/ScoopInstaller/Scoop#install-scoop-to-a-custom-directory-by-changing-scoop

If this variable is unset, Scoop defaults to C:\Users\<user>\scoop.

I was looking at this too, but as far as I understand (and I could be totally wrong here) the SCOOP variable is only set temporarily to configure the installation, but it does not persist.
However I honestly did not try it and maybe it would work that way and then I am sure this would be a much faster alternative.
If I find some time I will happily try this and if it works I will rewrite it.

@rashil2000
Copy link

I was looking at this too, but as far as I understand (and I could be totally wrong here) the SCOOP variable is only set temporarily to configure the installation, but it does not persist.

If you see the PowerShell command there:

[Environment]::SetEnvironmentVariable('SCOOP', $env:SCOOP, 'User')

this commands sets environment variables permanently (as opposed to just the current shell session, like on unix).

@FantasyTeddy
Copy link
Contributor Author

Ah that's the piece of information that was missing for me. Thank you very much for enlightening me :)
I will change the implementation accordingly and open another PR.

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

Successfully merging this pull request may close these issues.

None yet

3 participants