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

Renaming kebab-case cdylib target to kebab_case in metadata breaks cargo subcommands #13705

Open
workingjubilee opened this issue Apr 4, 2024 · 7 comments
Labels
C-bug Category: bug Command-metadata regression-from-stable-to-beta Regression in beta that previously worked in stable. S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.

Comments

@workingjubilee
Copy link
Member

workingjubilee commented Apr 4, 2024

Problem

Hi!

Previously for targets (possibly just cdylib targets?), cargo would emit an artifact.target.name that matched the name of the package! Now it uses the underscored name. This broke cargo-pgrx on beta, and it appears that it or a related change has broken at least one other cargo tool. I believe the breaking change is in the range of 7bb7b53...54d8815 since it appears on beta but not stable, for me.

I discovered this while hunting another bug or I would have figured it out much faster. As-is, it took me a while to narrow down this problem from amidst my other issues. If I can I will try to fix my other problems and bisect this to a specific cargo commit but I have a lot going on right now so! Though, maybe I will get lucky and this will somehow be the same issue?

It would be nice if this was publicized much more loudly but it doesn't seem to be in the CHANGELOG at that point in time. It might also be nice if I had other ways of finding the mapping of target -> output .so, .dylib, or .dll name than rummaging through the compiler's output messages or randomly picking over the output directory! I don't think I've ever seen documentation promising a particular output format...

If it's relevant, just to make things more confusing, this is a library that is both an rlib and cdylib!

Steps

No response

Possible Solution(s)

No response

Notes

No response

Version

$ cargo --version --verbose
cargo 1.78.0-beta.4 (54d8815d0 2024-03-26)
release: 1.78.0-beta.4
commit-hash: 54d8815d04fa3816edc207bbc4dd36bf18014dbc
commit-date: 2024-03-26
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.2 vendored)
libcurl: 8.6.0-DEV (sys:0.4.72+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Arch Linux Rolling Release [64-bit]
@workingjubilee workingjubilee added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Apr 4, 2024
@workingjubilee workingjubilee changed the title Renaming kebab-case cdylib target in metadata to kebab_case breaks cargo subcommands Renaming kebab-case cdylib target to kebab_case in metadata breaks cargo subcommands Apr 4, 2024
@epage epage added regression-from-stable-to-beta Regression in beta that previously worked in stable. Command-metadata labels Apr 4, 2024
@weihanglo
Copy link
Member

Isn't #12783 only available on nightly?

@epage
Copy link
Contributor

epage commented Apr 4, 2024

Correct, so something else must be going on.

@workingjubilee could you add reproduction steps?

@weihanglo
Copy link
Member

By just looking at linked issues in #12783, some folks are certainly bitten by cargo metadata output change 😞.

@workingjubilee
Copy link
Member Author

Unfortunately the "reproduction steps" seem to involve the enormity of the pgrx framework. I spent a bit on trying to rebuild a minimal version from scratch, and I seem to be unable to find a minimal reproducer that demonstrates that it happens on beta first, despite it most definitely happening anytime I have set rustup default beta! It's very weird. Unfortunately this has already consumed a large amount of time I should have been spending on hunting down a rustc bug that breaks my crate, so it will be a while before I can show you anything you can actually reason about.

I will however resume bisecting across rustc commits, and while I go at it, I can try to see if I can find the exact update to rustc where the breaks start happening for cargo-pgrx. Maybe then I will get some insight into extracting a reproducer.

@workingjubilee
Copy link
Member Author

workingjubilee commented Apr 5, 2024

Oh, I can add this much: Part of the reason I tried to go directly for a minimal reproducer instead of cutting things down in a more binary-search style is that by the time cargo build is run in this context, we have

  • started running cargo test
  • started executing test code
  • that then calls back into cargo pgrx
  • that then finally calls cargo build

...maybe there's an environment variable cargo previously set that is leading to this result. 🤔

@weihanglo weihanglo added S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. and removed S-triage Status: This issue is waiting on initial triage. labels Apr 6, 2024
@workingjubilee
Copy link
Member Author

Retrospectively, I think I misremembered, conflated, or misunderstood something amidst the hell of figuring out several things going wrong at once in the repository's test suite, so I think this "only" has the same cause as the other breakages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug Command-metadata regression-from-stable-to-beta Regression in beta that previously worked in stable. S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.
Projects
None yet
Development

No branches or pull requests

3 participants