-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Bump object and thorin-dwp #111413
Bump object and thorin-dwp #111413
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
cc @vladimir-ea |
This comment has been minimized.
This comment has been minimized.
ae6910d
to
5b00a88
Compare
This comment has been minimized.
This comment has been minimized.
5b00a88
to
7ebfe9e
Compare
This comment has been minimized.
This comment has been minimized.
7ebfe9e
to
325483c
Compare
Cargo.lock
Outdated
@@ -4621,7 +4648,7 @@ dependencies = [ | |||
"hermit-abi 0.3.0", | |||
"libc", | |||
"miniz_oxide", | |||
"object", | |||
"object 0.30.3", |
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.
Is depending on old version of object
for std
intentional?
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.
Yes, it's pointless to bump this unless backtrace is updated first.
There also exist #111384(or maybe one more?) with same bump, so need to coordinate. |
Does this PR contain all the version updates that #111384 needs? |
@petrochenkov Yes, that PR is now effectively a superset of this one on all points that matter. |
object -> 0.31.1 thorin-dwp -> 0.6.0 Required to fix watchOS breakage.
325483c
to
7156ff6
Compare
r=me when ready |
@Mark-Simulacrum sorry to jump into this thread. No sure but #111384 seems to already contain changes in this PR. |
Is ready, yes. @bors r=MarkSimulacrum |
@imWildCat It is the reviewer's intention to delay approving that PR until after this one is merged so we might as well get on with this one. |
☀️ Test successful - checks-actions |
@workingjubilee understand. Thank you! |
Excellent! I’ll rebase #111384 on top of this. |
Finished benchmarking commit (4eb5225): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 642.109s -> 645.683s (0.56%) |
I know no action was called for, but regarding the perf and 3.5 second bootstrap regression: 0.7 of these seconds is the object crate. Some of this is inevitable complication. However, with the enabling of compression features in object, it is possible that either object or thorin-dwp are including (thus, compiling) more code than Rust actually uses now, due to the use of compression by default. This could be fixed if thorin-dwp allowed forwarding features so we could disable the compression. However, it is likely we want to instead make more deliberate use of that compression somehow. |
also: Is await-call-tree normally a bouncy benchmark? The wall-time massively improved on that. |
…henkov Fix linking Mac Catalyst by including LC_BUILD_VERSION in object files Hello. My first rustc PR! Issue rust-lang#106021 prevents Rust code from being linked into Mac Catalyst applications. Apple's LD has started requiring object files to contain version information about the platform they were built for, such as: * the "deployment target" (minimum supported OS version), * the SDK version * the type of the platform (macOS/iOS/catalyst/tvOS/watchOS all have a different number). This is currently only enforced when building for Mac Catalyst. Rust uses the `object` crate which added support for including this information starting with `0.31.0`. ~~I upgraded it along with `thorin-dwp` so that everything depends on 0.31. Apparently 0.31 [pulls in](gimli-rs/object#463) `ruzstd` due to a [new ELF standard](https://maskray.me/blog/2022-09-09-zstd-compressed-debug-sections) because its `compression` feature is enabled by thorin. If you find this objectionable, let me know what the best way to avoid pulling in those dependencies might be.~~ **(`object` upgraded in rust-lang#111413 I then added two commits: * The first one adds very basic, hard-coded support for calling `set_macho_build_version` for `-macabi` (Catalyst) targets, where it claims deployment target of Catalyst 14.0 and SDK of 16.2. * The second weaves the versioning through `rust_target::spec::TargetOptions`, so that we can stick to specifying all target-related info in one place. Kudos to `@ara4n` for writing [this gist](https://gist.github.com/ara4n/320a53ea768aba51afad4c9ed2168536).
…henkov Fix linking Mac Catalyst by including LC_BUILD_VERSION in object files Hello. My first rustc PR! Issue rust-lang#106021 prevents Rust code from being linked into Mac Catalyst applications. Apple's LD has started requiring object files to contain version information about the platform they were built for, such as: * the "deployment target" (minimum supported OS version), * the SDK version * the type of the platform (macOS/iOS/catalyst/tvOS/watchOS all have a different number). This is currently only enforced when building for Mac Catalyst. Rust uses the `object` crate which added support for including this information starting with `0.31.0`. ~~I upgraded it along with `thorin-dwp` so that everything depends on 0.31. Apparently 0.31 [pulls in](gimli-rs/object#463) `ruzstd` due to a [new ELF standard](https://maskray.me/blog/2022-09-09-zstd-compressed-debug-sections) because its `compression` feature is enabled by thorin. If you find this objectionable, let me know what the best way to avoid pulling in those dependencies might be.~~ **(`object` upgraded in rust-lang#111413 I then added two commits: * The first one adds very basic, hard-coded support for calling `set_macho_build_version` for `-macabi` (Catalyst) targets, where it claims deployment target of Catalyst 14.0 and SDK of 16.2. * The second weaves the versioning through `rust_target::spec::TargetOptions`, so that we can stick to specifying all target-related info in one place. Kudos to `@ara4n` for writing [this gist](https://gist.github.com/ara4n/320a53ea768aba51afad4c9ed2168536).
…henkov Fix linking Mac Catalyst by including LC_BUILD_VERSION in object files Hello. My first rustc PR! Issue rust-lang#106021 prevents Rust code from being linked into Mac Catalyst applications. Apple's LD has started requiring object files to contain version information about the platform they were built for, such as: * the "deployment target" (minimum supported OS version), * the SDK version * the type of the platform (macOS/iOS/catalyst/tvOS/watchOS all have a different number). This is currently only enforced when building for Mac Catalyst. Rust uses the `object` crate which added support for including this information starting with `0.31.0`. ~~I upgraded it along with `thorin-dwp` so that everything depends on 0.31. Apparently 0.31 [pulls in](gimli-rs/object#463) `ruzstd` due to a [new ELF standard](https://maskray.me/blog/2022-09-09-zstd-compressed-debug-sections) because its `compression` feature is enabled by thorin. If you find this objectionable, let me know what the best way to avoid pulling in those dependencies might be.~~ **(`object` upgraded in rust-lang#111413 I then added two commits: * The first one adds very basic, hard-coded support for calling `set_macho_build_version` for `-macabi` (Catalyst) targets, where it claims deployment target of Catalyst 14.0 and SDK of 16.2. * The second weaves the versioning through `rust_target::spec::TargetOptions`, so that we can stick to specifying all target-related info in one place. Kudos to ``@ara4n`` for writing [this gist](https://gist.github.com/ara4n/320a53ea768aba51afad4c9ed2168536).
Use constants from object crate Replace hard-coded values with `GNU_PROPERTY_{X86|AARCH64}_FEATURE_1_AND` from the object crate. When working on [issue](rust-lang#103001) it was suggested that we moved these constants to the object crate . [PR](gimli-rs/object#537). Now that that the object crate has been updated [PR](rust-lang#111413) we can make this change.
Use constants from object crate Replace hard-coded values with `GNU_PROPERTY_{X86|AARCH64}_FEATURE_1_AND` from the object crate. When working on [issue](rust-lang#103001) it was suggested that we moved these constants to the object crate . [PR](gimli-rs/object#537). Now that that the object crate has been updated [PR](rust-lang#111413) we can make this change.
Required to fix watchOS breakage.