这是indexloc提供的服务，不要输入任何密码
Skip to content

enhance(main/rust): Suppress hard linking failed warning. #24947

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 4, 2025
Merged

enhance(main/rust): Suppress hard linking failed warning. #24947

merged 1 commit into from
Jun 4, 2025

Conversation

@MohammadKHC
Copy link
Contributor

@MohammadKHC MohammadKHC commented Jun 3, 2025

Well Android does not support hard linking. but rust does not know that and keeps filling the terminal with this useless warning.
This pr patches only one line from the upstream to suppress it.
Please refer to #4921.
See this image hard-linking

Note: this issue was reported to the upstream for almost 5 years. But unfortunately it seems that no one really cares about it. rust-lang/rust#75997

@TomJo2000 I followed the commit message guidelines this time ;)

TomJo2000
TomJo2000 reviewed Jun 3, 2025
View reviewed changes
Copy link
Member

@TomJo2000 TomJo2000 left a comment

Choose a reason for hiding this comment

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

Just to check.
This only turns off the warning being emitted by the compiler?
It does not mean the compiler will actually try to do hardlinks on Android.

@MohammadKHC
Copy link
Contributor Author

MohammadKHC commented Jun 3, 2025 •
edited
Loading

This only turns off the message.
The compiler by default tries to do hard linking and if it fails it shows this warning and still compile the code successfully.
This does not change that it justs doesn't log the message.

@TomJo2000
Copy link
Member

TomJo2000 commented Jun 3, 2025

Just wanted to make sure.

TomJo2000
TomJo2000 requested changes Jun 3, 2025
View reviewed changes
@MohammadKHC
Copy link
Contributor Author

MohammadKHC commented Jun 3, 2025

I think that it's alright now. Sorry it's actually my first day working with diffs.

@TomJo2000
Copy link
Member

TomJo2000 commented Jun 3, 2025

I think that it's alright now. Sorry it's actually my first day working with diffs.

No worries about that, that's what we have contribution resources and PR reviews for.

@TomJo2000
Copy link
Member

TomJo2000 commented Jun 3, 2025 •
edited
Loading

  • By the way I added another package linter check in,
    enhance(scripts/lint-packages): ensure version changes  #24948 based on your earlier PR.
    That way missing version bumps get caught before the package build instead of only getting caught in the upload test at the very end of the CI pipeline.
    (Coincidentally the way I'm checking for version bumps in that check is using diffs.)

@MohammadKHC
Copy link
Contributor Author

MohammadKHC commented Jun 3, 2025

Thank you. It seems like being dump helps somehow haha

@TomJo2000
Copy link
Member

TomJo2000 commented Jun 3, 2025

I'd say it made me aware of a gap in the linter coverage that should be addressed.

@MohammadKHC
Copy link
Contributor Author

MohammadKHC commented Jun 3, 2025

Oops (again) because of this change unused errors are triggered will fix that.

TomJo2000
TomJo2000 reviewed Jun 3, 2025
View reviewed changes
@MohammadKHC MohammadKHC force-pushed the master branch 3 times, most recently from cdd09ab to be2b632 Compare June 3, 2025 23:53
@MohammadKHC
Copy link
Contributor Author

MohammadKHC commented Jun 3, 2025

Well the third time is the charm. i am pretty sure that it's fixed now..

@MohammadKHC
Copy link
Contributor Author

MohammadKHC commented Jun 4, 2025

This pr patches only one line from the upstream to suppress it.

Famous last words. LOL
Fr i really think this is fixed now.

@MohammadKHC
Copy link
Contributor Author

MohammadKHC commented Jun 4, 2025

The CI succeeded 🎉

@TomJo2000
Copy link
Member

TomJo2000 commented Jun 4, 2025

I'd like to get @truboxl or @licy183 to sign off on this before merging this.
Just to make sure this doesn't have any unintended side effects.

licy183
licy183 approved these changes Jun 4, 2025
View reviewed changes
Copy link
Member

@licy183 licy183 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your contribution!

@TomJo2000 TomJo2000 merged commit 83cbedc into termux:master Jun 4, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TomJo2000 TomJo2000 TomJo2000 approved these changes

@licy183 licy183 licy183 approved these changes

@Grimler91 Grimler91 Awaiting requested review from Grimler91 Grimler91 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MohammadKHC @TomJo2000 @licy183