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.

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

Jump to bottom

Feature: Implement batch installation #216

Merged
merged 33 commits into from
Jul 20, 2022
Merged

Feature: Implement batch installation #216

merged 33 commits into from
Jul 20, 2022

Conversation

NobodyXu
Copy link
Member

Resolved #175

Signed-off-by: Jiahao XU [email protected]

NobodyXu added 24 commits July 18, 2022 16:05
@NobodyXu
derive(Clone) for DesiredTargets
9e1f873 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
derive(Clone) for CrateName
409f31f 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor main.rs: Extract new fn resolve
d6db552 
prepare for the new feature batch installation.

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor main.rs: Simplify install_from_source
730f7d6 
Rm arg `opts`

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Impl helpers::await_task: Handle JoinError
5e7aab7 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Support new feature batch installation!
7f11b74 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Avoid Box::clone for targets
40a872d 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor main.rs: Extract new fn install
d514219 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor: Avoid repeated heap alloc of temp_dir
8ca8538 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Optimize: Launch install immediately if confirmation is not required
90059c1 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor main.rs: Extract fn Resolution::print
f0fb7da 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Print out resolution in optimized path
c66d815 
where confirmation isn't required.

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Optimize main.rs: Avoid frequent Box::clone
bcb46cd 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor main.rs: Print resolution in resolve
119192f 
This simplified `entry`.

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor entry: Avoid Arc::clone
79ec122 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Add comment to entry to improve readbility
9552e0e 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Add new dep jobserver v0.1.24
8cc085b 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Impl new fn helpers::create_jobserver_client
c67c59b 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Use jobserver to limit parallism of cargo-install
fb0a6a5 
Since we execute multiple `cargo-install` concurrently, we must use
jobserver to limit the parallism so that they won't spawn too much
processes/threads to overload the system.

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Improve comment in create_jobserver_client
072253e 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Simplify helpers::await_task API
b223990 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Fix opts.no_cleanup behavior
c6281d8 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor: Simplify struct Resolution
b026462 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Test batch installtion in run_tests_unix.sh
d58f340 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu NobodyXu added the PR: feature work PR that provides or works on a new feature label Jul 18, 2022
@NobodyXu NobodyXu marked this pull request as ready for review July 18, 2022 15:01
@NobodyXu NobodyXu requested a review from passcod July 18, 2022 15:01
passcod
passcod reviewed Jul 18, 2022
View reviewed changes
src/main.rs Outdated Show resolved Hide resolved
@passcod
Copy link
Member

passcod commented Jul 18, 2022

This looks good apart from that the main.rs file is now very large and should be broken up into its component parts in the library. Ideally main.rs would be just args parsing + UI specifics + a few calls to the "backend".

@NobodyXu
Copy link
Member Author

This looks good apart from that the main.rs file is now very large and should be broken up into its component parts in the library. Ideally main.rs would be just args parsing + UI specifics + a few calls to the "backend".

Yes, I would post a PR to refactor this later.
The key is that resolve and install depends on information from Options, so we either move Options into the library as well, or we split it into multiple parts.

NobodyXu
NobodyXu commented Jul 19, 2022
View reviewed changes
src/main.rs Show resolved Hide resolved
@NobodyXu
Copy link
Member Author

@passcod It will be interesting to see whether mimalloc would speedup given enough crates specified, like 5/10/15.

I don't have the environment setup to benchmark it on my local machine and my network connection is slow (using 4G LTE modem).

@NobodyXu NobodyXu requested a review from passcod July 19, 2022 03:07
NobodyXu added 5 commits July 20, 2022 16:43
@NobodyXu
Add new dep once_cell v1.13.0
3961dbb 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor: Impl Source::cratesio_registry
5bdffd9 
Makes initializing `metafiles::CrateVersionSource` more readable and
improves performance since the parsing is now cached.

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor: Reduce params of install_from_package
1ebd4bd 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Rm unused param temp_dir from install_from_package
1eedae1 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Rm unnecessary clone in install_from_package
3b82e9e 
Signed-off-by: Jiahao XU <[email protected]>
@passcod
Copy link
Member

passcod commented Jul 20, 2022

The key is that resolve and install depends on information from Options,

true. I think putting these required data in another struct and then building that struct from clap options would work best.

passcod
passcod approved these changes Jul 20, 2022
View reviewed changes
Copy link
Member

@passcod passcod left a comment

Choose a reason for hiding this comment

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

Approving this, without a further refactor to simplify main.rs which will have to be done later.

@NobodyXu NobodyXu merged commit 3a30e87 into cargo-bins:main Jul 20, 2022
@NobodyXu NobodyXu deleted the feature/batch-installation branch July 20, 2022 07:18
This was referenced Jul 20, 2022
Merged
Merged
@passcod passcod mentioned this pull request Jul 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@passcod passcod passcod approved these changes

Assignees
No one assigned
Labels
PR: feature work PR that provides or works on a new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature: Batch installation
2 participants
@NobodyXu @passcod