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

Update cargo-marker and other CLI to enable general use #60

Open
11 of 14 tasks
xFrednet opened this issue Nov 24, 2022 · 0 comments
Open
11 of 14 tasks

Update cargo-marker and other CLI to enable general use #60

xFrednet opened this issue Nov 24, 2022 · 0 comments
Labels
A-marker-cargo Area: All things connected to `cargo_marker` C-tracking-issue Category: Tracking issue D-rustc-driver Driver: Rustc Driver

Comments

@xFrednet
Copy link
Member

xFrednet commented Nov 24, 2022 •
edited
Loading

With most of the item backend implemented, it's finally time to run it on the first real crates and see what errors, todos and limitations come up. This is a collection of several paper cuts I ran into. Most of these are specific for the cargo interface.

  • cargo linter passes linter as the second argument to cargo-linter which isn't handled correctly
  • cargo-linter uses nightly features, which means that cargo install ... would require a nightly toolchain, this should be avoidable, at least for the cargo subcommand crate.
  • cargo-linter has a dev feature, which is turned on by default (It should be off by default)
  • Rename the output binary to cargo-marker
  • Allow the specification of lint crates in a file (local path and git) #81
  • Allow fetching lints from registries #87
  • Lint crates should be allowed to have a different name than their directory.
    • This will require reading the metadata and using the defined name there
  • Lint crates should be specifiable in a file or something
  • Lint crates should use the marker_api version that the driver uses
    • The version could be overwritten during compilation with a command like this: cargo build --config "dependencies.marker_api=\"0.0.0\""
  • The user should be able to specify a custom driver version, this will imply the used toolchain and marker_api version
  • cargo-linter starts the driver with the toolchain from the local rust-toolchain which can cause errors. Until we have the preimage to be a component in rustup, we need to manually handle the toolchain for the driver
  • Store marker_rustc_driver with the toolchain, that it belongs to #142
  • Make cargo-marker handle OS strings correctly
    • It should be fine in most cases, but the test setup currently calls to_string_lossy
  • All of this needs to be documented.
@xFrednet xFrednet added C-enhancement Category: New feature or request A-marker-cargo Area: All things connected to `cargo_marker` D-rustc-driver Driver: Rustc Driver labels Nov 24, 2022
@xFrednet xFrednet added this to the v0.0.1 milestone Nov 24, 2022
@xFrednet xFrednet self-assigned this Nov 24, 2022
@xFrednet xFrednet mentioned this issue Nov 24, 2022
Merged
bors bot added a commit that referenced this issue Nov 26, 2022
@bors @xFrednet
Merge #62
c0572aa 
62: Minor cleanup in cargo-linter and stable CI r=xFrednet a=xFrednet

A small cleanup PR, not much to say, just a few nice changes, before I try to rework more things in `cargo-linter`

As always, happy about reviews, but I'll merge this, if there has been no progress by the next PR :)

---

Closes #61 and progress on #60

Co-authored-by: xFrednet <[email protected]>
@xFrednet xFrednet added C-tracking-issue Category: Tracking issue and removed C-enhancement Category: New feature or request labels Nov 28, 2022
@xFrednet xFrednet mentioned this issue Dec 1, 2022
Merged
@xFrednet xFrednet changed the title Update cargo-linter and other CI to enable general use Update cargo-marker and other CI to enable general use Dec 31, 2022
bors bot added a commit that referenced this issue Jan 5, 2023
@bors @xFrednet
Merge #66
0a449ed 
66: `cargo-marker` refactoring and review r=xFrednet a=xFrednet

I've refactored parts of cargo-marker and reviewed the implementation. I found several limitations which are now documented in #60. This PR already fixes a few of these.


CC: #60

---

Same as always: If I don't get a review, I'll merge this, when a depending PR is ready. :)


Co-authored-by: xFrednet <[email protected]>
@xFrednet xFrednet removed their assignment Jan 6, 2023
@xFrednet xFrednet mentioned this issue Jul 11, 2023
Merged
bors bot added a commit that referenced this issue Jul 12, 2023
@bors @xFrednet
Merge #168
bc5848e 
168: Cargo: Remove `cargo_fetch` dependency and rework the architecture r=xFrednet a=xFrednet

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

Co-authored-by: xFrednet <[email protected]>
@xFrednet xFrednet removed this from the v0.1.0 milestone Jul 16, 2023
@xFrednet xFrednet changed the title Update cargo-marker and other CI to enable general use Update cargo-marker and other CLI to enable general use Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-marker-cargo Area: All things connected to `cargo_marker` C-tracking-issue Category: Tracking issue D-rustc-driver Driver: Rustc Driver
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@xFrednet