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

Run cargo-deny at build (experimental/proposal) #148

Closed
wants to merge 1 commit into from

Conversation

kazuk
Copy link
Contributor

@kazuk kazuk commented May 18, 2022

Issue number and link

Fixes: #147

Describe your changes

add "springql-build" for build script crate.
this runs "cargo deny check" for workspace.

add springql-build to springql-core's [build-dependencies]

Checklist before requesting a review

  • I follow the Semantic Pull Requests rules (bugfix/feature)
  • I specified links to related issues (must: bugfix, want: feature)
  • I have performed a self-review of my code (bugfix/feature)
  • I have added thorough tests (bugfix/feature)
  • I have edited ## [Unreleased] section in CHANGELOG.md following keep a changelog syntax (bugfix/feature)
  • I {made/will make} a related pull request for documentation repo (feature)

@kazuk kazuk added the experimental proposal code for discussion label May 18, 2022
@laysakura
Copy link
Contributor

@kazuk Basically, I don't like complex build process.

However, I can agree with this pull-request, not only because
cargo deny check is not only helpful to improve developers' security but also light-weight command to execute.

We additionally need:

  • Documentation for developers (what we do in build process and why) in README or somewhere
  • Easy-to-revert merge commit for when a developer report some build trouble

@laysakura laysakura mentioned this pull request May 18, 2022
6 tasks
@codecov
Copy link

codecov bot commented May 20, 2022

Codecov Report

Merging #148 (11e9e58) into main (8b2cbf8) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #148      +/-   ##
==========================================
+ Coverage   87.79%   87.81%   +0.01%     
==========================================
  Files         203      204       +1     
  Lines       12620    12621       +1     
==========================================
+ Hits        11080    11083       +3     
+ Misses       1540     1538       -2     
Impacted Files Coverage Δ
springql-build/src/lib.rs 100.00% <100.00%> (ø)
...ne/autonomous_executor/row/column/stream_column.rs 97.43% <0.00%> (-0.52%) ⬇️
...omous_executor/row/value/sql_value/nn_sql_value.rs 67.02% <0.00%> (+0.54%) ⬆️
..._engine/autonomous_executor/row/value/sql_value.rs 75.51% <0.00%> (+0.68%) ⬆️
...ngql-core/src/stream_engine/autonomous_executor.rs 95.12% <0.00%> (+1.21%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b2cbf8...11e9e58. Read the comment docs.

@kazuk
Copy link
Contributor Author

kazuk commented May 23, 2022

Unfortunately, I have decided that this method will not work.

This method does not prevent the execution of malicious build scripts or procedural macros because the cargo build scripts are executed in parallel for the dependencies and the order is not guaranteed.

@kazuk kazuk closed this May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental proposal code for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ci: Prevent dependency issue for developers
2 participants