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

ci: 1.79 fixes (replace macos-latest stable with macos-latest 1.78) #1202

Merged
merged 16 commits into from
Jun 19, 2024

Conversation

dj8yfo
Copy link
Collaborator

@dj8yfo dj8yfo commented Jun 18, 2024

this passes if macos-latest is temporarily commented out https://github.com/near/near-sdk-rs/actions/runs/9566693799 ;
otherwise, macos-latest is having some serious trouble on 1.79

@ruseinov
Copy link
Collaborator

I'm looking into this in another branch and locally, if I make progress - I will let you know.

@ruseinov
Copy link
Collaborator

This is not reproducible locally on my mac. Could it be the cache?

@dj8yfo
Copy link
Collaborator Author

dj8yfo commented Jun 18, 2024

@ruseinov , what's the processor type of your machine? It might be some wicked bug of 1.79 affecting only aarch64-*-*, or it might be not

@ruseinov
Copy link
Collaborator

ruseinov commented Jun 18, 2024

@ruseinov , what's the processor type of your machine? It might be some wicked bug of 1.79 affecting only aarch64-*-*, or it might be not

I have an m2-pro, so that should have popped up.
Perhaps they have updated the OS? I'm still on Ventura. Unlikely though, 14 has been out for a while.

@ruseinov
Copy link
Collaborator

I have tried investigating this with macos-13 - that yields "test panicked while processing panic". There might indeed be something wrong with the setup somewhere.

@ruseinov
Copy link
Collaborator

ruseinov commented Jun 18, 2024

I guess you have already talked to @frol. Perhaps we should pin the version to 1.78 as a temp fix, either via rust-toolchain.toml or just via CI. I see no reason investigating that as it does not reproduce easily on any other environments.

@dj8yfo
Copy link
Collaborator Author

dj8yfo commented Jun 18, 2024

@ruseinov created an issue, as my somewhat limited research hasn't been particularly useful

@ruseinov
Copy link
Collaborator

@ruseinov created an issue, as my somewhat limited research hasn't been particularly useful

That’s fine, let’s so 1.78 to fix master for now. And solve the issue later, unless it solves itself with the next version. Can’t be debugging machine-specific failures with actual access to the machine.

@dj8yfo dj8yfo changed the title ci: 1.79 fixes ci: 1.79 fixes (replace macos-latest stable with macos-latest 1.78) Jun 19, 2024
@dj8yfo dj8yfo marked this pull request as ready for review June 19, 2024 07:38
@dj8yfo dj8yfo requested review from frol, agostbiro and uint as code owners June 19, 2024 07:38
frol
frol previously approved these changes Jun 19, 2024
@@ -18,12 +18,15 @@ pub enum BindgenArgType {
/// A single argument of a function after it was processed by the bindgen.
pub struct ArgInfo {
/// Attributes not related to bindgen.
#[allow(unused)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these really unused or is it a clippy bug?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as far as go-to-references have told me, these are unused. looks like the code was copied from elsewhere,
but only some of the fields are used in the context of near-sdk-macros

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

triple-checked these; they are only created inside of ::new constructors, and not touched by other code thereafter. So, as far as tooling can tell, these are unused

.github/workflows/test.yml Outdated Show resolved Hide resolved
…github/workflows/test.yml`

Co-authored-by: Vlad Frolov <frolvlad@gmail.com>
@frol frol merged commit 5970226 into near:master Jun 19, 2024
15 checks passed
@frol frol mentioned this pull request Jun 19, 2024
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