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

badtouch: add --locked flag to cargo install #46182

Closed

Conversation

chenrui333
Copy link
Member

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Relates to #46025

@samford
Copy link
Member

samford commented Nov 3, 2019

The CI build is failing here because the lock file in badtouch v0.7.0 (latest) uses an old version of kuchiki, which in turn uses an old version of cssparser that has a build error.

Dependency versions were bumped in the latest commit to badtouch (2019-08-13), which updates the kuchiki version and fixes the cssparser error. However, the changes to src/ulimit.rs in that commit lead to a mismatched type error with Rust 1.38.0 on my machine (macOS Catalina 10.15.1) but this error didn't appear in the upstream Travis CI builds using 1.38.0.

error[E0308]: mismatched types
  --> src/ulimit.rs:13:21
   |
13 |     RLIMIT_NOFILE = libc::RLIMIT_NOFILE,
   |                     ^^^^^^^^^^^^^^^^^^^ expected u32, found i32
help: you can convert an `i32` to `u32` and panic if the converted value wouldn't fit
   |
13 |     RLIMIT_NOFILE = libc::RLIMIT_NOFILE.try_into().unwrap(),
   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.

Simply running cargo update with the v0.7.0 files leads to a successful build locally (albeit with more warnings), since the changes to src/ulimit.rs aren't included. This is something that will need to be addressed upstream and it may be good to have them bump the dependency versions again and create a new release at the same time, if it's not too much trouble.

They may also want to fix the clippy warnings, since they're using a function (std::mem::uninitialized) in an unsafe block in src/ulimit.rs that will be deprecated in 1.39.0.

@chenrui333
Copy link
Member Author

I have the same exact error on my catalina machine as well.

@chenrui333
Copy link
Member Author

chenrui333 commented Nov 3, 2019

Based the PR build (https://travis-ci.org/kpcyrd/badtouch/builds/606802583) results, I think it just did not quite work on their end.

image

@samford
Copy link
Member

samford commented Nov 3, 2019

Those macOS builds fail with the same error (error[E0308]: mismatched types), which is expected since your PR only updates the dependencies and doesn't address the source of the error on macOS (src/ulimit.rs), as described above. The owner will need to decide how to fix the ulimit error on macOS but it would be good if you mentioned the error and provided the full error text so they're aware of the problem.

@samford
Copy link
Member

samford commented Nov 8, 2019

The build error we were encountering related to src/ulimit.rs has been fixed upstream (kpcyrd/authoscope#73) and is in their master branch. It looks like we'll have to wait until the next release after 0.7.0 to add the --locked flag.

Besides that, the author of badtouch was looking for your input on their PR to add macOS to their Travis setup (kpcyrd/authoscope#74), based on your PR (kpcyrd/authoscope#72).

@kpcyrd
Copy link
Contributor

kpcyrd commented Nov 9, 2019

I've tagged 0.7.1 that should resolve this, please let me know if you need anything else. Thanks for your work!

@chenrui333
Copy link
Member Author

@kpcyrd, cool, let me check on the new release.

Sorry for the late reply on the PR (this week was crazy for me).

@chenrui333
Copy link
Member Author

It works good now. Thanks @kpcyrd @samford !!

@chenrui333 chenrui333 closed this in 9421a6f Nov 9, 2019
@lock lock bot added the outdated PR was locked due to age label Jan 2, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 2, 2020
@chenrui333 chenrui333 deleted the badtouch-add-locked-flag branch December 18, 2022 04:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants