-
Notifications
You must be signed in to change notification settings - Fork 28
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
Initial cargo support #808
Conversation
9e08277
to
adefbb4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Straightforward basic Rust support. LGTM.
I had to add one more commit to update test data for gomod to pass the CI. |
Signed-off-by: Michal Šoltis <[email protected]>
Signed-off-by: Michal Šoltis <[email protected]>
3508452
to
5f891f3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address the commentary nitpicks before merging :) .
except tomli.TOMLDecodeError as e: | ||
log.debug("Parsing pyproject.toml at %r", str(self._setup_file)) | ||
return tomlkit.parse(self._setup_file.path.read_text()) | ||
except Exception as e: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering why they haven't exposed their top-level TOMLKitError in the public API reference, that's weird. I'm tempted to say "it's been stable since its introduction in 2018 so let's use it", but let's keep this as is until the team responds to my issue: python-poetry/tomlkit#399.
Just add a "TODO" commentary here and mention we'd want to replace Exception with a finer grained one once a resolution is reached in python-poetry/tomlkit#399.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just use from tomlkit.exceptions import TOMLKitError
? I don't see any exception being exposed in the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just use from tomlkit.exceptions import TOMLKitError ?
Like I said earlier
I'm tempted to say "it's been stable since its introduction in 2018 so let's use it (i.e. TOMLKitError)"
but even that exception is not part of their public API so technically can change, though I don't think it will.
Install cargo in the builder stage and copy the binary to the final image to keep the size as minimal as possible. Signed-off-by: Michal Šoltis <[email protected]>
Since Rust ecosystem uses TOML files: - Cargo.toml - Cargo.lock (also TOML) - .cargo/config.tom Let's install robust library that has more functionality than just load/loads that is provided by `tomli` package that we already have as a project dependency. I found `tomlkit` as a recommended libary for editing already existing TOML files [1]. + Regenerate requirements files. --- [1]: https://docs.python.org/3/library/tomllib.html Signed-off-by: Michal Šoltis <[email protected]>
Add logic to resolve each Cargo package by vendoring its dependencies (crates) into the output directory using Cargo CLI. The important option here is `--locked` that disabled any changes to Cargo.lock file. To use vendored sources, `.cargo/config.toml` must contain the following lines. See [1] for more details. Replace hardcoded directory path with a placeholder for output directory specified by the users when running `cachi2 inject-files ...` ``` [source.crates-io] replace-with = "vendored-sources" [source.vendored-sources] directory = "${path-to-vendored-sources}" ``` --- [1]: https://crates.io/crates/cargo-vendor Signed-off-by: Michal Šoltis <[email protected]>
Remove tomli from dependencies as it is no longer needed, while tomlkit was already used in other parts of the codebase. Signed-off-by: Michal Šoltis <[email protected]>
Resolved last comments |
Maintainers will complete the following section
Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:
/ok-to-test
(as is the standard for Pipelines as Code)