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

🐛 Memory allocation error for non root user #1712

Open
rljohnsn opened this issue May 19, 2024 · 13 comments
Open

🐛 Memory allocation error for non root user #1712

rljohnsn opened this issue May 19, 2024 · 13 comments

Comments

@rljohnsn
Copy link

rljohnsn commented May 19, 2024

After install a non root user gets a memory error. Running as root works fine.

❯ sudo delta
[sudo] password for lance:
The main way to use delta is to configure it as the pager for git: see https://github.com/dandavison/delta#get-started. You can also use delta to diff two files: `delta file_A file_B`.
❯ ls -laF /usr/bin/delta
-rwxr-xr-x 1 root root 7059472 Mar 16 07:28 /usr/bin/delta*
❯ ldd /usr/bin/delta
        linux-vdso.so.1 (0x00007ffc8659b000)
        libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f9701de0000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f9701dc0000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f9701cd9000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f9701ab0000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f97024c2000)
❯ delta
memory allocation of 1970324836974592 bytes failed
[1]    11340 IOT instruction  delta
❯ sudo delta
The main way to use delta is to configure it as the pager for git: see https://github.com/dandavison/delta#get-started. You can also use delta to diff two files: `delta file_A file_B`.
❯ uname -a
Linux DESKTOP-UE7J8VS 5.15.146.1-microsoft-standard-WSL2 #1 SMP Thu Jan 11 04:09:03 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
❯ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 22.04.4 LTS
Release:        22.04
Codename:       jammy

Windows 11, WSL2, Ubuntu 22.04

Delta version: https://github.com/dandavison/delta/releases/download/0.17.0/git-delta_0.17.0_amd64.deb

@dandavison
Copy link
Owner

dandavison commented May 20, 2024

Hi @rljohnsn, this memory allocation error has come up several times before in delta issues: https://github.com/dandavison/delta/issues?q=is%3Aissue+%22memory+allocation%22

It seems that it's always something to do with stale syntect syntax definitions. Can you try some of the remedies in those threads, such as installing bat and

bat cache --clear
bat cache --build

@ernstki
Copy link
Contributor

ernstki commented Jan 13, 2025

In my situation, re-running bat cache --build causes the core dump to reoccur consistently.

This is with bat 0.18.1, which, if I had to guess, is probably pretty old. Is this a bat or a delta bug, and how could I provide more information that would help one project or the other get to the bottom of this?

I do have a custom syntax for LDIF in ~/.config/bat/syntaxes, but I've move that and ~/.config/bat/config out of the way in order to eliminate some variables while troubleshooting. Same result.

@dandavison
Copy link
Owner

This is with bat 0.18.1, which, if I had to guess, is probably pretty old.

What if you didn't have to guess? :)

@ernstki
Copy link
Contributor

ernstki commented Jan 13, 2025

Because I was pretty sure my guess was right. :)

This seemed like something you might like (me) to dig into, while I have the conditions set for a reproducible crash, since it appears to be a recurring problem.

If your preferred solution is "just update bat to the latest version," I understand.

@dandavison
Copy link
Owner

Haha, thanks! It would be great to be able to state in the docs which version of bat delta requires. Hopefully it is largely predictable from semver semantics. I haven't checked, but suspect that the code we're using to read these bat assets is supplied by the bat library itself. So hopefully the answer is just that one must use a bat executable version that is, according to semver, compatible with the bat library version used by delta. Of course bat, like, delta, is still <v1 so every minor version may be a breaking change w.r.t. binary asset compat.

@ernstki
Copy link
Contributor

ernstki commented Jan 13, 2025

but suspect that the code we're using to read these bat assets is supplied by the bat library itself. So hopefully the answer is just that one must use a bat executable version that is, according to semver, compatible with the bat library version used by delta

OK, I can confirm that if I have bat available in my search path at the version requested in Cargo.toml, then bat cache --clear && bat cache --build, I'm back in business.

$ curl -s https://raw.githubusercontent.com/dandavison/delta/refs/tags/0.18.1/Cargo.toml | grep ^bat
bat = { version = "0.24.0", default-features = false, features = [

$ type -a bat
bat is /some/path/modules/third-party/rust/1.84.0/CARGO_HOME/bin/bat
bat is /some/other/path/opt/bin/bat

$ bat --version
bat 0.24.0 (b8b8d15-modified)

$ bat cache --clear
Clearing theme set cache ... okay
Clearing syntax set cache ... okay
Clearing metadata file ... okay

$ bat cache --build
No themes were found in '/my/home/.config/bat/themes', using the default set
Writing theme set to /my/home/.cache/bat/themes.bin ... okay
Writing syntax set to /my/home/.cache/bat/syntaxes.bin ... okay
Writing metadata to folder /my/home/.cache/bat ... okay

$ delta --version
delta 0.18.2

So your theory checks out. Thanks!

@ernstki
Copy link
Contributor

ernstki commented Jan 13, 2025

@dandavison Would you like a PR to update the docs, and if so, where at? The bottom of "Installation"?

I suppose that would help distro packagers, too, assuming they look there. Otherwise I could foresee a situation where bat and delta's packages have different maintainers, and one becomes out-of-sync with the other.

Would it be worth it to have delta itself check… somehow… that the bat in the search path is a compatible version? Seems like a headache.

@dandavison
Copy link
Owner

The bottom of "Installation"?

That sounds good, thanks. If you do it, can you make it clear that bat is not required by delta. Rather it's that if you are using custom bat themes/syntaxes, then you must use a compatible bat version, since delta will automatically find your custom themes/syntaxes.

@ernstki
Copy link
Contributor

ernstki commented Jan 13, 2025

if you are using custom bat themes/syntaxes

It's entirely possible I was just sloppy and forgot a bat cache --clear or something, but I'm not sure it's even a requirement to have a custom theme or syntax. I had emptied out my ~/.config/bat directory, cleared and rebuilt the bat cache, and (I think) I was still seeing the delta crash.

I'll confirm one way or the other with a more careful test, and report on whatever I find in the PR.

@dandavison
Copy link
Owner

dandavison commented Jan 13, 2025

The only connection between delta and bat is that delta uses the binary assets distributed by the bat project, and also some functions from the bat Rust crate related to invoking a pager. delta does not use the bat executable or require that it's installed; the delta executable contains its own copy of all the binary assets. I'm pretty sure that the only point of contact is that, if you've built custom bat assets using the bat executable, then delta will find them.

@ernstki
Copy link
Contributor

ernstki commented Jan 13, 2025

Rather it's that if you are using custom bat themes/syntaxes, then you must use a compatible bat version, since delta will automatically find your custom themes/syntaxes.

I think a possible refinement of this statement would be "if you are using custom bat themes/syntaxes, or have voluntarily created a theme/syntax cache using bat cache --build," as in:

$ rm -rf ~/.cache/bat
$ bat cache --build
No themes were found in '/home/me/.config/bat/themes', using the default set
No syntaxes were found in '/home/me/config/bat/syntaxes', using the default set.
Writing theme set to /home/me/.cache/bat/themes.bin ... okay
Writing syntax set to /home/me/.cache/bat/syntaxes.bin ... okay
Writing metadata to folder /home/me/.cache/bat ... okay

Apart from that, we are in accord!

It's not likely anyone will have done this intentionally unless they had custom themes or syntaxes, but the mere presence of a ~/.cache/bat generated by an incompatible version appears to be enough to crash delta, regardless of whether it contained any custom themes or syntaxes.

@dandavison
Copy link
Owner

Right, agreed, your version is more accurate.

ernstki added a commit to ernstki/delta that referenced this issue Jan 14, 2025
- make explicit mention of which bat version should be installed in the
  "Installation" and "Supported languages and themes" sections of the
  manual

- add missing "Tips & tricks" page

See dandavison#1712 for context.
@ernstki
Copy link
Contributor

ernstki commented Jan 14, 2025

Bat at v0.18.3 seems to be where the problems arise.

Screenshot of the output of the below Makefile, demonstrating a failure at the point where it tests bat v0.18.3

Here's the Makefile I used to make this determination. I tested using the musl release tarball for every release of delta back to 0.16.5, and the crash occurs consistently with bat v0.18.3 or earlier.

ernstki added a commit to ernstki/delta that referenced this issue Jan 14, 2025
- make explicit mention of which bat version should be installed in the
  "Installation" and "Supported languages and themes" sections of the
  manual

- add missing "Tips & tricks" page

See dandavison#1712 for context.
dandavison pushed a commit that referenced this issue Jan 14, 2025
- make explicit mention of which bat version should be installed in the
  "Installation" and "Supported languages and themes" sections of the
  manual

- add missing "Tips & tricks" page

See #1712 for context.
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

No branches or pull requests

3 participants