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

Cargo: Remove cargo_fetch dependency and rework the architecture #168

Merged

Conversation

xFrednet
Copy link
Member

This PR exploded a bit 😅. Removing the dependency required a few more changes than expected. The good news are, that marker now uses a stable way to fetch lint crates which also supports registries directly. No more cargo dependency, just command calls. I'm happy :D

It also addresses or at least touches several other issues. Generally speaking, a nice update IMO. (The only big negative thing is the error handling. I've never learned how to do that properly, and cargo is getting more messy with less information after each refactoring 😅. I'll create an issue for that :))


Closes: #167
Closes: #155
Closes: #87

CC: #60

@xFrednet xFrednet added C-enhancement Category: New feature or request A-marker-cargo Area: All things connected to `cargo_marker` A-tests Area: Tests, this includes the testing framework and existing tests S-waiting-on-review Status: Awaiting review labels Jul 11, 2023
@xFrednet xFrednet added this to the v0.1.0 milestone Jul 11, 2023
@xFrednet
Copy link
Member Author

Btw, let's make sure it works on every system

bors try

bors bot added a commit that referenced this pull request Jul 11, 2023
@bors
Copy link
Contributor

bors bot commented Jul 11, 2023

try

Build failed:

@xFrednet
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Jul 11, 2023
@bors
Copy link
Contributor

bors bot commented Jul 11, 2023

try

Build failed:

@xFrednet xFrednet force-pushed the 167-cargo-fetch-crates-something-something branch from e719e11 to ed7783b Compare July 11, 2023 21:59
@xFrednet
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Jul 11, 2023
@bors
Copy link
Contributor

bors bot commented Jul 11, 2023

try

Build failed:

@xFrednet xFrednet force-pushed the 167-cargo-fetch-crates-something-something branch from ed7783b to 73a2f8b Compare July 11, 2023 22:06
@xFrednet
Copy link
Member Author

Windows...

bors try

bors bot added a commit that referenced this pull request Jul 11, 2023
@bors
Copy link
Contributor

bors bot commented Jul 11, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@xFrednet
Copy link
Member Author

I noticed that the cargo-marker as a library for UI tests approach won't work for out of tree tests... Looks like I'll just have to revert to the old test setup for now :/

@xFrednet xFrednet marked this pull request as draft July 11, 2023 22:18
@xFrednet
Copy link
Member Author

And once again

bors try

@xFrednet xFrednet removed the A-tests Area: Tests, this includes the testing framework and existing tests label Jul 11, 2023
bors bot added a commit that referenced this pull request Jul 11, 2023
@bors
Copy link
Contributor

bors bot commented Jul 11, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@xFrednet xFrednet force-pushed the 167-cargo-fetch-crates-something-something branch from fd601ad to 58a53b2 Compare July 12, 2023 16:34
@xFrednet
Copy link
Member Author

This was a fight... and still is... but for now:

bors try

bors bot added a commit that referenced this pull request Jul 12, 2023
I still like the idea of using `cargo-marker` as a library. However, this would limit the normal driver discovery mechanism for external uitests. This is not to say it can't be done, just that this is a good fix in the meantime.
@bors
Copy link
Contributor

bors bot commented Jul 12, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@xFrednet
Copy link
Member Author

I think now it works... Damn that was a fight...

@xFrednet xFrednet force-pushed the 167-cargo-fetch-crates-something-something branch from 58a53b2 to d58e9d0 Compare July 12, 2023 18:11
Toolchains are pain, it would be so awesome if marker could just be a part of rustup
@xFrednet xFrednet force-pushed the 167-cargo-fetch-crates-something-something branch from d58e9d0 to af6d518 Compare July 12, 2023 23:02
@xFrednet xFrednet marked this pull request as ready for review July 12, 2023 23:02
@xFrednet
Copy link
Member Author

xFrednet commented Jul 12, 2023

I'm sure this is still broken somehow, but everything seems to be working. So let's follow the: "What's the worst that can happen ¯\_(ツ)_/¯" philosophy and merge this. I have another branch, which requires the changes from this one and we can always update the code later :)

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 12, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit bc5848e into rust-marker:master Jul 12, 2023
3 checks passed
@xFrednet xFrednet deleted the 167-cargo-fetch-crates-something-something branch July 12, 2023 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-marker-cargo Area: All things connected to `cargo_marker` C-enhancement Category: New feature or request S-waiting-on-review Status: Awaiting review
Projects
None yet
1 participant