Skip to content

Fix compilation bug in toolchain >= 1.70.0#321

Closed
yasuo-ozu wants to merge 1 commit intoPyO3:mainfrom
yasuo-ozu:main
Closed

Fix compilation bug in toolchain >= 1.70.0#321
yasuo-ozu wants to merge 1 commit intoPyO3:mainfrom
yasuo-ozu:main

Conversation

@yasuo-ozu
Copy link
Contributor

Fix #320

@adamreichold
Copy link
Member

Is this behaviour on a path towards stabilization? Should we already commit to this approach?


quiet = self.qbuild or ext.quiet
debug = self._is_debug_build(ext)
is_new_toolchain = get_rust_version() >= Version('1.70.0-nightly')
Copy link
Member

Choose a reason for hiding this comment

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

I think a more description name would be helpful (today's new is tomorrow's old). Also do we need to include "nightly" here? Can we compare directly against "1.70.0" or this considered larger than "1.70.0-nightly" by Version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. I will withdraw this PR and fix it in another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote Version('1.70.0-nightly') here, because I want is_new_toolchain to be True when get_rust_version() == Version("1.70.0-nightly").
But in fact Version('1.70.0-nightly') >= Version('1.70.0') is False.

Do you think something like the following is better?

rustc_version = get_rust_version()
is_new_toolchain = rustc_version.major > 1 or (rustc_version.major == 1 and rustc_version.minor >= 70)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think that would be preferable. (Especially if combined with a more descriptive name than "new".)

@yasuo-ozu
Copy link
Contributor Author

I have mistaken the merging branch, so I will close this PR once.

@yasuo-ozu yasuo-ozu closed this Mar 29, 2023
yasuo-ozu added a commit to yasuo-ozu/setuptools-rust that referenced this pull request Mar 29, 2023
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.

Compilation in setuptools_rust fails with 1.70.0-nightly toolchain

2 participants