Query crate versions from local registry index rather than HTTP API #317

DCjanus · 2019-06-27T06:41:34Z

Close #311

src/registry.rs

src/fetch.rs

DCjanus · 2019-07-16T16:30:58Z

@ordian Hi, since your last comment, I did a lot of changes, would you mind review again?

recommend: review one commit by one commit.

ordian

This is great work, @DCjanus, thank you!
Looks good overalls, but the thing that worries me here is a lack of tests. Do you think it would be possible to add some?

src/fetch.rs

src/errors.rs

src/fetch.rs

src/registry.rs

src/fetch.rs

src/registry.rs

src/fetch.rs

src/registry.rs

src/fetch.rs

src/errors.rs

DCjanus · 2019-07-20T16:03:31Z

force push: resolve conflict

ordian · 2019-07-21T16:23:00Z

It seems like appveyor CI is failing because libgit2 doesn't work on pc-windows-gnu targets.

The code looks ok, but I'd like get more reviews on this. @killercup @bjgill

bjgill · 2019-07-24T16:36:38Z

(I'll put this on my list of things to review; will try and take a look this week)

bjgill

Done a brief first pass. I really like the idea of speeding cargo-edit up and stopping using the HTTP API. Do you know how big the speed-up is?

A few smaller comments below.

As Appveyor identified, this change means that cargo-edit will no longer work with pc-windows-gnu. Is this acceptable/is the number of users small? I'm a bit nervous about dropping support for a tier 1 toolchain.

src/errors.rs

src/fetch.rs

DCjanus · 2019-07-25T17:58:38Z

I believe the problem on Windows-gnu could be resolved, I'm going to try this in this week.

DCjanus · 2019-07-26T15:36:38Z

My friend has told me how to fix this problem: rust-lang/rust#53454

And maybe, we could query registry index via git-cli rather than libgit2, with this, I believe that cost less compile time. But this requires the user to have to install git, I am not sure if this is acceptable.

For example:

$ git rev-parse refs/remotes/origin/master
e000bd939a83dfbb64da285425186af30178042b

$ git ls-tree e000bd939a83dfbb64da285425186af30178042b
040000 tree 2e03f39bea7b299346dd61f76223b0622a5c3ca5    1
040000 tree 766307154d671917d2ae02ca00035e7d7a96b711    2
040000 tree 62382fdc0c2d60f11eb1ffb4c9382bd360415625    3
040000 tree ea15ada1fa58ff60180e5768fc096279038d0fab    a-
040000 tree 669a2b1afa7d00b43c2e0eb72af40f85d6608993    aa
040000 tree d10ac8d5d8de21947dcd5a8bf7b19203480a80c1    ab
.... # some output

$ git cat-file -p 999e8ba108a3d80be1c24a4323e05fce422e2533 # a blob object hash that I found in my registry index
{"name":"dargo","vers":"0.0.0","deps":[],"cksum":"25ccfb6ab26454d5bd4c6a1391bb97a920ad0bfef13a25d5125e3e2ba8a6ebb1","features":{},"yanked":false,"links":null}
{"name":"dargo","vers":"0.0.1","deps":[{"name":"cargo","req":"^0.35.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"colored","req":"^1.8.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"failure","req":"^0.1.5","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"flexi_logger","req":"^0.13.1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"log","req":"^0.4.6","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"semver","req":"^0.9.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"structopt","req":"^0.2.16","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"tabwriter","req":"^1.1.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"toml_edit","req":"^0.1.3","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"}],"cksum":"babf4f39a8d8e51ce26cca6f3481401a7db37b3c8944f07407b69aaab9d0ce2e","features":{},"yanked":false,"links":null}
{"name":"dargo","vers":"0.0.2","deps":[{"name":"cargo","req":"^0.35.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"colored","req":"^1.8.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"failure","req":"^0.1.5","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"flexi_logger","req":"^0.13.1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"log","req":"^0.4.6","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"semver","req":"^0.9.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"structopt","req":"^0.2.16","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"tabwriter","req":"^1.1.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"toml_edit","req":"^0.1.3","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"}],"cksum":"c499b31da448619f627099e8f9951d49d80639f5e622d73832f86ad899454953","features":{},"yanked":false,"links":null}
{"name":"dargo","vers":"0.0.3","deps":[{"name":"cargo","req":"^0.35.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"colored","req":"^1.8.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"failure","req":"^0.1.5","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"flexi_logger","req":"^0.13.1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"log","req":"^0.4.6","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"semver","req":"^0.9.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"structopt","req":"^0.2.16","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"tabwriter","req":"^1.1.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"toml_edit","req":"^0.1.3","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"}],"cksum":"769f10a0e29b4a97e81555d06b2dae04e80d0cf76911c37609a938f35259f8be","features":{},"yanked":false,"links":null}
{"name":"dargo","vers":"0.0.4","deps":[{"name":"cargo","req":"^0.36.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"colored","req":"^1.8.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"failure","req":"^0.1.5","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"flexi_logger","req":"^0.13.2","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"log","req":"^0.4.6","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"semver","req":"^0.9.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"structopt","req":"^0.2.17","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"tabwriter","req":"^1.1.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"toml_edit","req":"^0.1.3","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"}],"cksum":"a4f5474ec1099fc7a8f57632027c8f3f42edbba9f5bfd83fa7894361d5aaa991","features":{},"yanked":false,"links":null}
{"name":"dargo","vers":"0.0.5","deps":[{"name":"cargo","req":"^0.36.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"colored","req":"^1.8.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"failure","req":"^0.1.5","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"flexi_logger","req":"^0.13.2","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"log","req":"^0.4.6","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"semver","req":"^0.9.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"structopt","req":"^0.2.16","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"tabwriter","req":"^1.1.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"toml_edit","req":"^0.1.3","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"}],"cksum":"670e5c55a81f5693c33360d8fb84b4f1a0f166aceea977cb571142e2f0f283b2","features":{},"yanked":true,"links":null}
{"name":"dargo","vers":"0.0.6","deps":[{"name":"cargo","req":"^0.36.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"colored","req":"^1.8.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"failure","req":"^0.1.5","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"flexi_logger","req":"^0.13.2","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"log","req":"^0.4.6","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"semver","req":"^0.9.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"structopt","req":"^0.2.16","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"tabwriter","req":"^1.1.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"toml_edit","req":"^0.1.3","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"}],"cksum":"12675eb23f091fcc331d58bb0a6dc84702a2529fd333456db5d86d33e8aaafda","features":{},"yanked":false,"links":null}

ordian · 2019-07-26T15:55:05Z

As Appveyor identified, this change means that cargo-edit will no longer work with pc-windows-gnu. Is this acceptable/is the number of users small? I'm a bit nervous about dropping support for a tier 1 toolchain.

This is a valid concern, but I think the number of pc-windows-gnu target's users is pretty small (they could probably use pc-windows-msvc as well) and would be in favor of dropping support for it for now and let the issue be fixed upstream (in rust/libgit2) rather than require a user to have git installed and run it in CLI mode.
Let me know if you don't agree.

DCjanus · 2019-07-26T16:07:32Z

Fair enough to me, maybe we should recommend Windows users to install with msvc.

And before upstream fix this problem, and if I have enough free time, I'd like to try parse .git by hand, with that, less compile time and less dependencies.

If I have enough free time……

bjgill · 2019-08-01T16:40:32Z

OK, thanks. I'm happy for this to go in as-is, then. You'll just need to remove the gnu targets from https://github.com/killercup/cargo-edit/blob/master/appveyor.yml#L12 to get CI to pass.

…local registry' or 'registry index in local filesystem'

… future maintainers

remove windows-gnu targets in CI; add missing doc for foreign_links errors; upgrade dependencies;

DCjanus · 2019-08-07T09:59:30Z

@ordian Hi, I just resloved some conflict and force pushed, would you mind take a look.

ordian · 2019-08-07T10:30:24Z

@DCjanus hey, thank you!

Travis CI failure on beta seems spurious, so let's get this merged.
bors r+

317: Query crate versions from local registry index rather than HTTP API r=ordian a=DCjanus Close #311 Co-authored-by: DCjanus <dcjanus@dcjanus.com>

bors · 2019-08-07T10:55:47Z

Build succeeded

ordian reviewed Jun 30, 2019

View reviewed changes

src/registry.rs Outdated Show resolved Hide resolved

ordian reviewed Jun 30, 2019

View reviewed changes

src/fetch.rs Outdated Show resolved Hide resolved

DCjanus changed the title ~~[WIP] Query crate versions from local registry index rather than HTTP API~~ Query crate versions from local registry index rather than HTTP API Jul 16, 2019

ordian reviewed Jul 17, 2019

View reviewed changes

ordian reviewed Jul 19, 2019

View reviewed changes

src/registry.rs Outdated Show resolved Hide resolved

src/fetch.rs Outdated Show resolved Hide resolved

src/errors.rs Outdated Show resolved Hide resolved

DCjanus mentioned this pull request Jul 20, 2019

refactor: remove error wraps #320

Closed

bjgill self-requested a review July 24, 2019 16:36

bjgill reviewed Jul 25, 2019

View reviewed changes

src/errors.rs Outdated Show resolved Hide resolved

src/fetch.rs Show resolved Hide resolved

src/fetch.rs Show resolved Hide resolved

DCjanus changed the title ~~Query crate versions from local registry index rather than HTTP API~~ WIP: Query crate versions from local registry index rather than HTTP API Jul 25, 2019

DCjanus changed the title ~~WIP: Query crate versions from local registry index rather than HTTP API~~ Query crate versions from local registry index rather than HTTP API Aug 3, 2019

DCjanus added 11 commits August 7, 2019 17:53

Query crate versions from local registry index rather than HTTP API

f18498d

Fix unit test: error message

5f5fbe3

Impl registry index short name construct

987db95

reduce wildcard number limit; reduce code by 'to_le_bytes'

8afe825

better name: 'local registry index' might be understood as 'index of …

acda9ff

…local registry' or 'registry index in local filesystem'

add unit test for 'short_name'; add cargo code url

717298e

feature: source replace support

1cdf22a

feature: index update support

d44de40

feature: offline flag

14d7505

upgrade dependencies(serde, serde_derive): 1.0.94 -> 1.0.97

834a970

documentation: better error message and description; some comment for…

f2c355f

… future maintainers

DCjanus added 14 commits August 7, 2019 17:53

remove wildcard number limit

d3dc2cf

check empty crate name

c3f2bbb

fix clippy warning: clippy::try_err

a762cf9

fix test "add_typo"

4aedd94

add some test cases

359cff2

remove git gc stuff

4d90a49

fix typo(CargoConfig): 'sources' -> 'source'

33c7d63

replace current directory with manifest path

813ec71

update registry index on the begin

51e39f4

fix: fuzzy match origin crate first

e33244a

update dependencies

1207f96

remove unnecessary display

1fc9b56

some mirror change

96c5e7a

remove windows-gnu targets in CI; add missing doc for foreign_links errors; upgrade dependencies;

update dependencies

1193902

bors bot added a commit that referenced this pull request Aug 7, 2019

Merge #317

af51449

317: Query crate versions from local registry index rather than HTTP API r=ordian a=DCjanus Close #311 Co-authored-by: DCjanus <dcjanus@dcjanus.com>

bors bot merged commit 1193902 into killercup:master Aug 7, 2019

killercup mentioned this pull request Aug 17, 2019

Release 0.4 #332

Merged

This was referenced Aug 25, 2019

Respect source replacement configuration #269

Closed

Upgrade workspaces more efficiently (caching and parallelisation) #158

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Query crate versions from local registry index rather than HTTP API #317

Query crate versions from local registry index rather than HTTP API #317

DCjanus commented Jun 27, 2019

DCjanus commented Jul 16, 2019

ordian left a comment

DCjanus commented Jul 20, 2019

ordian commented Jul 21, 2019

bjgill commented Jul 24, 2019

bjgill left a comment

DCjanus commented Jul 25, 2019

DCjanus commented Jul 26, 2019

ordian commented Jul 26, 2019

DCjanus commented Jul 26, 2019

bjgill commented Aug 1, 2019 •

edited

Loading

DCjanus commented Aug 7, 2019

ordian commented Aug 7, 2019

bors bot commented Aug 7, 2019

Query crate versions from local registry index rather than HTTP API #317

Query crate versions from local registry index rather than HTTP API #317

Conversation

DCjanus commented Jun 27, 2019

DCjanus commented Jul 16, 2019

ordian left a comment

Choose a reason for hiding this comment

DCjanus commented Jul 20, 2019

ordian commented Jul 21, 2019

bjgill commented Jul 24, 2019

bjgill left a comment

Choose a reason for hiding this comment

DCjanus commented Jul 25, 2019

DCjanus commented Jul 26, 2019

ordian commented Jul 26, 2019

DCjanus commented Jul 26, 2019

bjgill commented Aug 1, 2019 • edited Loading

DCjanus commented Aug 7, 2019

ordian commented Aug 7, 2019

bors bot commented Aug 7, 2019

Build succeeded

bjgill commented Aug 1, 2019 •

edited

Loading