Skip to content

Do not override the normal algorithm for finding config.toml files#384

Open
netsweng wants to merge 4 commits into
mainfrom
user/netsweng/pathtoconfig
Open

Do not override the normal algorithm for finding config.toml files#384
netsweng wants to merge 4 commits into
mainfrom
user/netsweng/pathtoconfig

Conversation

@netsweng
Copy link
Copy Markdown

Do not override the normal algorithm for finding config.toml files as the 2 choices encoded here do not cover all situations. Instead, leave the default to let cargo do it's normal thing, but add a --config option so pipelines can pass something in for unusual cases.

Copilot AI review requested due to automatic review settings May 12, 2026 20:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates cargo xtask precheck to stop hardcoding a .cargo/config.toml lookup path and instead rely on Cargo’s default config discovery, while adding a --config passthrough so CI/pipelines can supply an explicit Cargo config override when needed.

Changes:

  • Removed custom .cargo/config.toml path probing logic from precheck.
  • Added a --config CLI option to precheck and wired it into the Setup subtask.
Comments suppressed due to low confidence (1)

xtask/src/precheck.rs:129

  • self.config is moved into Setup (config: self.config), which partially moves self and will prevent later use of other non-Copy fields (e.g., self.exclude, self.package, etc.) in this function. Clone or take the value (e.g., config: self.config.clone() or fn run(mut self, ..) + self.config.take()) to avoid a borrow-checker error.
        if stage.setup || stage.all {
            Setup {
                force: false,
                config: self.config,
                skip_taplo: self.skip_taplo,
                skip_audit: self.skip_audit,
                skip_openssl: self.skip_openssl,
            }
            .run(ctx.clone())?;

Comment thread xtask/src/precheck.rs Outdated
Copilot AI review requested due to automatic review settings May 12, 2026 21:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@netsweng netsweng requested a review from jaygmsft May 12, 2026 21:47
Stuart R. Anderson and others added 2 commits May 13, 2026 10:49
as the 2 choices encoded here do not cover all situations. Instead,
leave the default to let cargo do it's normal thing, but add a
--config option so pipelines can pass something in for unusual cases.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@netsweng netsweng force-pushed the user/netsweng/pathtoconfig branch from 3e4a9f5 to ece9204 Compare May 13, 2026 17:49
@netsweng netsweng requested a review from rajesh-gali May 13, 2026 17:50
@netsweng netsweng enabled auto-merge (squash) May 13, 2026 21:51
Copilot AI review requested due to automatic review settings May 13, 2026 21:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread xtask/src/precheck.rs
Comment thread xtask/src/precheck.rs
Setup {
force: false,
config: Some(config_path),
config: self.config,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will users need to pass this config when running precheck either when enlisted with azihsm-sdk or its other counterpart location?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly in the other location. Default in azihsm-sdk seem to do the right thing already.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to ensure that cargo xtask precheck can work on any location that we regularly use for development without having to pass additional arguments manually.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pipelines can pass overrides and that is ok, user shouldnt have to pass it for local runs on devbox

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config.toml is in the expected location withing the azihsm-sdk tree. By removing the hardcoded paths here, the default behavior of walking up the tree as many levels as is needed should once again be in effect.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread xtask/src/precheck.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants