Skip to content

jemalloc configuration may be broken #17168

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

Closed
vhbit opened this issue Sep 11, 2014 · 9 comments · Fixed by #17196
Closed

jemalloc configuration may be broken #17168

vhbit opened this issue Sep 11, 2014 · 9 comments · Fixed by #17196

Comments

@vhbit
Copy link
Contributor

vhbit commented Sep 11, 2014

92b0926 broke iOS build.

It happened because a new jemalloc branch was started over 2014-05-08 instead of 2014-06-05. While the new branch has all the required features, they are disabled because configure script is bundled.

So either configure should be removed completely and constructed by autoconf during build process as usually or it should be regenerated and checked in on the every jemalloc update. The latter perhaps requires a guide for contributors so they to be aware of the issue.

@vhbit
Copy link
Contributor Author

vhbit commented Sep 11, 2014

cc @thestinger @alexcrichton

@alexcrichton
Copy link
Member

What precisely is broken? Can iOS just not build? Why? Also, what do you mean by the May 8 and June 5 dates?

We currently don't have a dependence on autotools, and there are certainly many of us that'd like to keep it that way! Do we just need to check in a new configure script?

@vhbit
Copy link
Contributor Author

vhbit commented Sep 11, 2014

Can iOS just not build? Why?

jemalloc fails to build as old configure script from 2014-05-08 doesn't contain support for iOS.

Also, what do you mean by the May 8 and June 5 dates?

Sorry for not being clear enough, those are names of jemalloc snapshot branches

Do we just need to check in a new configure script?

Yep, that should be enough

@alexcrichton
Copy link
Member

If you'd like, it's ok to disable jemalloc by default for ios (for rust). Otherwise this is why we prefer to upstream changes before including them in our own repos (for example rust-lang/jemalloc@5921ba7).

@vhbit
Copy link
Contributor Author

vhbit commented Sep 11, 2014

The fun thing is that actually changes are upstreamed you can see if you scroll a bit in 2014-09-09. But upstream doesn't contain configure, this file is presented only in Rust repo and therefore it is fragile on jemalloc updates - rebase on wrong branch and it may be invalid, this very case.

@alexcrichton
Copy link
Member

Aha! Well in that case, feel free to submit an updated configure script!

@vhbit
Copy link
Contributor Author

vhbit commented Sep 11, 2014

Will do. My point for creating this issue is increasing awareness of problem for future jemalloc updates

@thestinger
Copy link
Contributor

I regenerated the configure script with autoconf when I made that branch and it does contain the ios case in the case statement.

@vhbit
Copy link
Contributor Author

vhbit commented Sep 12, 2014

Guys, I terribly sorry and apologize, but it is my fault in the end - it seems that iOS was deleted from upstream, from config.sub file which caused invalid configure file to be generated. Looks like a mistake as iOS was deleted in commit related to OpenRISC cleanup. I've sent a new PR to jemalloc and will update once it lands.

Apologies again.

This was referenced Sep 12, 2014
lnicola pushed a commit to lnicola/rust that referenced this issue May 19, 2024
…r=lnicola

Update `rust-analyzer` to use `windows-sys` crate

I noticed that the `rust-analyzer` project already depends on `windows-sys`. This update merely replaces the remaining direct dependencies on the older `winapi` crate with `windows-sys` dependencies.

Originally posted here: rust-lang#124578
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 a pull request may close this issue.

3 participants