Some maintenance #371
Some maintenance #371
Conversation
The `proc-macro-error` crate is deprecated, hence use `proc-macro-error2` instead. Closes #370.
There was a more recent release which needs a different config. Also make it as strict as it was intended by the original author. And finally use the official GitHub Action to run it on CI.
| @@ -13,7 +13,7 @@ proc-macro = true | |||
| [dependencies] | |||
| proc-macro2 = { version = "1.0.24", features = ["span-locations"] } | |||
| proc-macro-crate = "3.1.0" | |||
| proc-macro-error = "1.0.4" | |||
| proc-macro-error2 = "2.0.1" | |||
There was a problem hiding this comment.
This crate doesn't seem too popular. What's the issue with keeping the old one?
There was a problem hiding this comment.
The reason is https://rustsec.org/advisories/RUSTSEC-2024-0370.html. And if you run something like cargo-deny (which we do on CI), it will complain about it. And I would expect that maybe someone will complain, hence it's easier to upgrade to that one. Another point is that crates are moving on to syn v2, so you would've less duplicated deps as proc-macro-error is still using v1.
There was a problem hiding this comment.
I'd be more concerned about using a random crate than using a so-called "unmaintained" crate, from a supply-chain perspective at least.
There was a problem hiding this comment.
The supply-chain perspective is a good point. I'll update that PR when I find time.
There was a problem hiding this comment.
(I have to wonder if we could just vendor it)
There was a problem hiding this comment.
If you find it annoying, please complain to the rustsec project.
There was a problem hiding this comment.
To expand on that, it's one thing if an unmaintained project has security issues piling up. But jumping to the latest hot fork is just as likely to cause problems as not. Declaring a project unmaintained without a clear replacement is literally just dangling a vulnerable part of the OSS supply chain in front of potential attackers and asking them to "jump in and take a bite".
There was a problem hiding this comment.
https://rustsec.org/advisories/RUSTSEC-2024-0370.html lists 3 replacements, including proc-macro-error2 used in this PR. And it does pull outdated version of syn that is one more thing to compile for everyone downstream.
So overall I think it is fair to warn users that this crate is unmaintained and if they have any issues with it in the future, there will be no easy solution for them, which is even worse when dependency is indirect.
There was a problem hiding this comment.
https://rustsec.org/advisories/RUSTSEC-2024-0370.html lists 3 replacements, including proc-macro-error2 used in this PR.
I'm talking about clear replacements. In this case, both the advisory and the suggestion to use proc-macro-error2 were submitted by the author of proc-macro-error2.
The other alternatives were added later, but still aren't clear replacements.
So overall I think it is fair to warn users that this crate is unmaintained and if they have any issues with it in the future, there will be no easy solution for them, which is even worse when dependency is indirect.
RUSTSEC is a security project and treats these advisories as security advisories. Sure, I get warning about unmaintained security critical bits if there is a clear maintained replacement, but reports like this one just cause people to complain and are more likely to cause security issues than they are to resolve them.
There was a problem hiding this comment.
Ok, here's a version that simply drops the offending dependency: #372.
|
Merged by rebase in #372. |
This is a PR that bundles some maintenance work. With all these changes, the dependencies are up to date and the CI is green. It doesn't contain any major changes.
This PR shouldn't be squashed when merged as it's really individual changes only bundles as they are so small.