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

rust 1.52.1 #76780

Closed
wants to merge 2 commits into from
Closed

rust 1.52.1 #76780

wants to merge 2 commits into from

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented May 6, 2021

Created with brew bump-formula-pr.

@EricFromCanada
Copy link
Member

Heads-up that the SDKROOT block can be removed. Its absence allows the current version to build on 10.12 and doesn't appear to affect later versions.

@cho-m cho-m mentioned this pull request May 10, 2021
@carlocab
Copy link
Member

1.52.1 has been released.

@disrupted

This comment has been minimized.

@Bo98
Copy link
Member Author

Bo98 commented May 10, 2021

Well you still have to fix the breakages caused by 1.52, if you're happy to do that go ahead. I won't be able to look into it until tomorrow.

disrupted added a commit to disrupted/homebrew-core that referenced this pull request May 10, 2021
@disrupted
Copy link
Contributor

Well you still have to fix the breakages caused by 1.52, if you're happy to do that go ahead. I won't be able to look into it until tomorrow.

I see what you mean now. Was hoping for a quick fix to get the update for rust-analyzer through. But in that case I am going to close my PR as I think it will make more sense to make the necessary changes directly here if you can bump to v1.52.1

@carlocab
Copy link
Member

Was hoping for a quick fix to get the update for rust-analyzer through.

Rust takes two days to run in CI, so the version bump was never going to be quick, unfortunately.

@cho-m
Copy link
Member

cho-m commented May 10, 2021

Starting some initial failure analysis for Intel nodes.

Rust failures (i.e. may be due to formula bump):

  • angle-grinder - actual Rust compilation failure. probably need to analyze further:
       Compiling ag v0.16.0 (/private/tmp/angle-grinder-20210506-90401-nbddyx/angle-grinder-0.16)
    error: proc-macro derive panicked
      --> src/alias.rs:13:26
       |
    13 | const ALIASES_DIR: Dir = include_dir!("aliases");
       |                          ^^^^^^^^^^^^^^^^^^^^^^^
       |
       = help: message: assertion failed: source.starts_with(prefix)
       = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

Failures probably not specific to this formula:

@carlocab
Copy link
Member

carlocab commented May 10, 2021

vice is a test-bot bug. asuka is only on Big Sur and doesn't have a Big Sur bottle, so we can ignore that one.

Does hrydgard/ppsspp#14176 apply cleanly to the current version in Homebrew/core?

@cho-m
Copy link
Member

cho-m commented May 10, 2021

@carlocab probably worth a try in PR as I was able to git apply ... on top of git checkout v1.11.3

@carlocab
Copy link
Member

This is the freeswitch error:

In file included from locks/unix/thread_mutex.c:17:
In file included from /private/tmp/freeswitch-20210507-44211-eeq084/libs/apr/include/arch/unix/apr_arch_thread_mutex.h:27:
In file included from /private/tmp/freeswitch-20210507-44211-eeq084/libs/apr/include/arch/unix/apr_private.h:881:
In file included from /private/tmp/freeswitch-20210507-44211-eeq084/libs/apr/include/arch/unix/../apr_private_common.h:24:
In file included from ./include/apr_pools.h:39:
In file included from ./include/apr_general.h:33:
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/signal.h:69:42: error: use of undeclared identifier 'NSIG'
extern __const char *__const sys_signame[NSIG];
                                         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/signal.h:70:42: error: use of undeclared identifier 'NSIG'
extern __const char *__const sys_siglist[NSIG];
                                         ^

Logs: freeswitch.tar.gz

@cho-m
Copy link
Member

cho-m commented May 11, 2021

Then the freeswitch failure may be the same issue discussed in closed attempt to bump to 1.10.6 (#75728) relating to upstream failure from signalwire/freeswitch#1145.

@carlocab
Copy link
Member

Is freeswitch building its own apr? Maybe we can persuade it to use the brewed one, if it is?

@cho-m
Copy link
Member

cho-m commented May 11, 2021

Not too sure, but configure.ac apr related section appears to be:
https://github.com/signalwire/freeswitch/blob/v1.10.5/configure.ac#L835-L838

AC_CHECK_LIB(apr-1, apr_pool_mutex_set, use_system_apr=yes, use_system_apr=no)
AM_CONDITIONAL([SYSTEM_APR],[test "${use_system_apr}" = "yes"])
AC_CHECK_LIB(aprutil-1, apr_queue_pop_timeout, use_system_aprutil=yes, use_system_aprutil=no)
AM_CONDITIONAL([SYSTEM_APRUTIL],[test "${use_system_aprutil}" = "yes"])

Probably need to check Homebrew's apr has apr_pool_mutex_set.

EDIT: Doesn't seem to be available in headers for Homebrew's or CLT's version of apr.
May be a function added into a forked apr
freeswitch code: https://github.com/signalwire/freeswitch/blob/v1.10.5/libs/apr/include/apr_pools.h#L427-L428

@cho-m cho-m mentioned this pull request May 11, 2021
5 tasks
@cho-m
Copy link
Member

cho-m commented May 11, 2021

For chronograf (and similar Node 16 failures from #76791 (comment)) is it better to:

  • leave build failure?
  • downgrade to node@14
  • try to yarn upgrade node-sass@6.0.0 (only version that currently supports Node 16. The risk is that chronograf uses node-sass@4.13.0 so it is a pretty large upgrade).

@SMillerDev
Copy link
Member

For chronograf (and similar Node 16 failures from #76791 (comment)) is it better to

I'd say downgrade node

@cho-m
Copy link
Member

cho-m commented May 11, 2021

angle-grinder does --build-from-source with Rust 1.51.0.

So, that failure may actually be related to bump to Rust 1.52

@Bo98
Copy link
Member Author

Bo98 commented May 11, 2021

Yeah, I have a patch for that I'll test later today. I think it requires bumping proc-macro-hack from 0.4.2 to 0.4.3.

The rest aren't really blockers since they can be proven to not be Rust-related, but of course fixing them is ideal.

iMichka added a commit to iMichka/homebrew-core that referenced this pull request May 27, 2021
@cho-m cho-m mentioned this pull request May 28, 2021
@carlocab carlocab changed the title rust 1.52.0 rust 1.52.1 May 31, 2021
@BrewTestBot BrewTestBot added the bump-formula-pr PR was created using `brew bump-formula-pr` label May 31, 2021
@carlocab carlocab added the in progress Stale bot should stay away label May 31, 2021
@carlocab
Copy link
Member

Let's give this another go.

Bo98 and others added 2 commits May 31, 2021 11:45
Also:

- remove the `SDKROOT` block
- scope `MACOSX_DEPLOYMENT_TARGET` to macOS

`SDKROOT` is no longer needed, and breaks the build on 10.12.
@cho-m
Copy link
Member

cho-m commented May 31, 2021

angle-grinder is going to still fail to compile with Rust 1.52 since the lock file hasn't changed.

I guess there is an option to ignore failure for now and deal with it afterward.

@carlocab
Copy link
Member

carlocab commented May 31, 2021

ARM failures with bottles:

@carlocab
Copy link
Member

Ok, the mpv build is just haunted

@carlocab
Copy link
Member

carlocab commented Jun 1, 2021

Mojave

Error: 13 failed steps!
brew install --build-from-source angle-grinder
brew test --retry --verbose buku
brew install --build-from-source cargo-audit
brew test --retry --verbose cargo-watch
brew test --retry --verbose cargo-watch
brew install --build-from-source gifski
brew install --build-from-source graph-tool
brew test --retry --verbose ktmpl
brew install --build-from-source openimageio
brew install --build-from-source pdfpc
brew install --build-from-source vice
brew test --retry --verbose wasm-pack
brew test --retry --verbose wasm-pack

@cho-m
Copy link
Member

cho-m commented Jun 2, 2021

Ignoring some standard failures and those analyzed for ARM, the remaining ones for Intel are:

  • openimageio (All, build)
  • buku (Mojave, test)
  • deno (Big Sur, build) - LLVM ERROR: IO failure on output stream: No space left on device

@carlocab
Copy link
Member

carlocab commented Jun 2, 2021

Can't reproduce the openimageio failure from current master.

Could it be a fluke? I don't think openimageio even needs rust at runtime.

@cho-m
Copy link
Member

cho-m commented Jun 2, 2021

Not sure why openimageio started failing. It didn't fail in the previous run, but it did fail in the Linux-specific PR #78107, which didn't touch the Rust version.

It's possibly due to something else changing in build environment (e.g. other formulae or CI settings).

There is a Homebrew error on possible token expiration, though I haven't checked if this is cause of failure:

/usr/local/Homebrew/Library/Homebrew/utils/github/api.rb:290:in `raise_error': GitHub API Error: Bad credentials (GitHub::API::AuthenticationFailedError)
HOMEBREW_GITHUB_API_TOKEN may be invalid or expired; check:
  https://github.com/settings/tokens
	from /usr/local/Homebrew/Library/Homebrew/utils/github/api.rb:234:in `open_rest'
	from /usr/local/Homebrew/Library/Homebrew/utils/github.rb:166:in `search'

@carlocab
Copy link
Member

carlocab commented Jun 2, 2021

Yea, seems like a network error. Probs didn't get to do git clone properly or something.

We could try re-running to see if it was just a fluke. That's pretty costly, though. And I doubt any change in rust can actually break openimageio without also breaking the intermediate formulae that connect openimageio to rust in the dependency graph.

@cho-m
Copy link
Member

cho-m commented Jun 3, 2021

I also didn't see an issue building both rust 1.52.1 and openimageio locally.

I will go with my previous comment that angle-grinder is probably the only formula affected by this update.
As rust is its only dependency, I don't expect us needing to rebuild the bottles.
If we need to fix, we can try modifying the problematic dependency that @Bo98 was looking into.

As more Rust projects are starting to use 1.52+ features, I think it is better to get this merged.

@carlocab
Copy link
Member

carlocab commented Jun 3, 2021

Yes, let's merge this. It's blocking a few other PRs.

@BrewTestBot
Copy link
Member

:shipit: @carlocab has triggered a merge.

@chenrui333 chenrui333 removed the in progress Stale bot should stay away label Jun 4, 2021
@cho-m cho-m mentioned this pull request Jun 20, 2021
@github-actions github-actions bot added the outdated PR was locked due to age label Jul 9, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bump-formula-pr PR was created using `brew bump-formula-pr` outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants