security_audit CI check is failing on main

[alamb]

Describe the bug

We are seeing a cargo audit failure on @zebsme 's PR: #15454

Crate:     proc-macro-error
Version:   1.0.4
Warning:   unmaintained
Title:     proc-macro-error is unmaintained
Date:      2024-09-01
ID:        RUSTSEC-2024-0370
URL:       https://rustsec.org/advisories/RUSTSEC-2024-0370
Dependency tree:
proc-macro-error 1.0.4
└── structopt-derive 0.4.18
    └── structopt 0.3.26
        └── datafusion-benchmarks 46.0.1

error: 1 vulnerability found!
warning: 3 allowed warnings found

The error is actually happening on main as well, but the CI job is only setup to run when Cargo.toml/Cargo.lock changes:

datafusion/.github/workflows/audit.yml

Lines 25 to 33 in ac919d5

	push:
	paths:
	- "**/Cargo.toml"
	- "**/Cargo.lock"
	pull_request:
	paths:
	- "**/Cargo.toml"
	- "**/Cargo.lock"

The job can start failing when a new entry is added to the database, in addition to when the crates used by datafusion are changed

To Reproduce

# in datafusion directory
cargo audit

Expected behavior

No response

Additional context

No response

[Jiashu-Hu]

take

[Jiashu-Hu]

For now, it might be best to just allow this warning. Since everything is working fine as it is. Alternatively we could try to use 'proc-macro-error2' instead, but that might cause new problems because it's not exactly the same as the old one.

If you agree with this, I'll submit a PR to add this warning to the allowlist for now.

Found a PR from another community that someone trying to replace it with 'proc-macro-error2': yewstack/yew#3752

[jayzhan211]

Can we try proc-macro-error2? Unmaintained crate isn't the prefer one to rely on. If it is not trivial or causing too much breaking change, then we can consider adding a warning instead

[alamb]

Another approach would be to remove the use of structopt in datafusion benchmarks (and update them to use clap directly...)

While more work this would likely be an easier to maintain long term solution

https://crates.io/crates/structopt

The docs page basically says it is in maintenance mode; https://docs.rs/structopt/0.3.26/structopt/#maintenance

As clap v3 is now out, and the structopt features are integrated into (almost as-is), structopt is now in maintenance mode: no new feature will be added.

[Jiashu-Hu]

Another approach would be to remove the use of structopt in datafusion benchmarks (and update them to use clap directly...)

While more work this would likely be an easier to maintain long term solution

https://crates.io/crates/structopt

The docs page basically says it is in maintenance mode; https://docs.rs/structopt/0.3.26/structopt/#maintenance

As clap v3 is now out, and the structopt features are integrated into (almost as-is), structopt is now in maintenance mode: no new feature will be added.

Using proc-macro-error2 could introduce new issues that are tough to resolve, based on the PR I mentioned earlier. Since we got from their experience, I think we may try replace structopt with clap, I will start working on that

[alamb]

I agree that removing structopt is the best approach

To fix the CI check, perhaps we can temporarily ignore that warning as it relates to a tool that is not published as part of DataFusion

[Jiashu-Hu]

Great, I will go ahead submit a PR to ignore it first

[alamb]

Thank you so much @Jiashu-Hu