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

v0.18.3 not building with Rust 1.57 on Linux x64 #1991

Closed
Kage-Yami opened this issue Dec 26, 2021 · 4 comments
Closed

v0.18.3 not building with Rust 1.57 on Linux x64 #1991

Kage-Yami opened this issue Dec 26, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@Kage-Yami
Copy link

Describe the bug you encountered:

Not able to build version 0.18.3 using cargo install bat on Rust 1.57.0, x86_64-unknown-linux-gnu. (Originally encountered when trying to install cargo-expand, but this happens when trying to install just bat as well.)

❯ cargo install bat
...
error[E0599]: no method named `add_from_folder` found for struct `ThemeSet` in the current scope
  --> /home/yami/.cargo/registry/src/github.com-1ecc6299db9ec823/bat-0.18.3/src/assets.rs:59:33
   |
59 |             let res = theme_set.add_from_folder(&theme_dir);
   |                                 ^^^^^^^^^^^^^^^ method not found in `ThemeSet`

For more information about this error, try `rustc --explain E0599`.
error: failed to compile `bat v0.18.3`, intermediate artifacts can be found at `/tmp/cargo-installpeRs2H`

Caused by:
  could not compile `bat` due to previous error

What did you expect to happen instead?

bat builds without error.

How did you install bat?

cargo install bat

bat version and environment

❯ rustc -Vv   
rustc 1.57.0 (f1edd0429 2021-11-29)
binary: rustc
commit-hash: f1edd0429582dd29cccacaf50fd134b05593bd9c
commit-date: 2021-11-29
host: x86_64-unknown-linux-gnu
release: 1.57.0
LLVM version: 13.0.0
@Kage-Yami Kage-Yami added the bug Something isn't working label Dec 26, 2021
@Enselic
Copy link
Collaborator

Enselic commented Dec 26, 2021

That's because a new version of syntect was released with changed feature set

cargo install --locked bat as recommended in readme should work

@Emoun
Copy link

Emoun commented Dec 26, 2021

I had the same problem as @Kage-Yami and can confirm that using cargo install --locked with bat and cargo-expand solved the issue.

@Enselic, I tried to look at syntect feature set and can see they added the feature plist-load, but cannot see how that should result in breaking bat's build. Could you elaborate?

Also, this makes 4.7.0 of syntect incompliant with semver right?

lastly, does anyone know why lockfiles aren't used by default when present for binaries?

@ehuss
Copy link

ehuss commented Dec 26, 2021

@Enselic Regarding trishume/syntect#345, would you possibly consider reverting that change? Moving existing public APIs behind a feature is a semver-breaking change, and generally shouldn't be done in a minor release.

Emoun added a commit to Emoun/duplicate that referenced this issue Dec 26, 2021
The updated nightly version fixes a bug with the
'match_block_trailing_comma' rustfmt config.
Previously not all missing commas were caught.

When installing cargo-expand, we now use '--locked' to ensure
lock-files are used.
This is a workaround for the release of v4.7.0 of the 'syntect' crates,
which violated semantic versioning. Using the lock files avoids this
version. For details see sharkdp/bat#1991.
Emoun added a commit to Emoun/duplicate that referenced this issue Dec 26, 2021
The updated nightly version fixes a bug with the
'match_block_trailing_comma' rustfmt config.
Previously not all missing commas were caught.
Probably related to rust-lang/rustfmt#4998.

When installing cargo-expand, we now use '--locked' to ensure
lock-files are used.
This is a workaround for the release of v4.7.0 of the 'syntect' crates,
which violated semantic versioning. Using the lock files avoids this
version. For details see sharkdp/bat#1991.
@Enselic
Copy link
Collaborator

Enselic commented Dec 26, 2021

This is an oversight on my part. Since syntect v4.7.0 has been yanked now there is no action to take for bat. A fix will be made in syntect, tracked in trishume/syntect#402. So we can close this issue.

Thank you for quickly bringing this issue to our attention.

@Enselic Enselic closed this as completed Dec 26, 2021
Enselic pushed a commit to Enselic/syntect that referenced this issue Dec 28, 2021
It is not enough to make the `default` feature depend on it. Putting code behind
a feature is in practice a semver breakage. See
sharkdp/bat#1991 (comment) and trishume#402.

It was added in trishume#345 after doing some refactoring. We can keep the refactoring,
but we choose to remove the feature for now. Another option would be to make the
feature opt-out rather than opt-in and rename it to `no-plist-load` or similar.
That would be too unergonomic and confusing however, so for now we just remove
it. We might add it back for v5.0.0
Enselic pushed a commit to Enselic/syntect that referenced this issue Dec 28, 2021
It was not enough to make the `default` feature depend on it. Putting code
behind a feature is in practice a semver breakage. See
sharkdp/bat#1991 (comment) and trishume#402.

It was added in trishume#345 after doing some refactoring. We can keep the refactoring,
but we choose to remove the feature for now. Another option would be to make the
feature opt-out rather than opt-in and rename it to `no-plist-load` or similar.
That would be too unergonomic and confusing however, so for now we just remove
it. We might add it back for v5.0.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants