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

fd -l/--list-details doesn't work in busybox #783

Closed
peternewman opened this issue Jun 27, 2021 · 9 comments · Fixed by #819
Closed

fd -l/--list-details doesn't work in busybox #783

peternewman opened this issue Jun 27, 2021 · 9 comments · Fixed by #819

Comments

@peternewman
Copy link
Contributor

Describe the bug you encountered:

Try to use fd -l in my specific case fd --extension json --list-details

ls: unrecognized option: human-readable
BusyBox v1.32.1 () multi-call binary.

Usage: ls [-1AaCxdLHRFplinshrSXvctu] [-w WIDTH] [FILE]...

List directory contents

	-1	One column output
	-a	Include entries which start with .
	-A	Like -a, but exclude . and ..
	-x	List by lines
	-d	List directory entries instead of contents
	-L	Follow symlinks
	-H	Follow symlinks on command line
	-R	Recurse
	-p	Append / to dir entries
	-F	Append indicator (one of */=@|) to entries
	-l	Long listing format
	-i	List inode numbers
	-n	List numeric UIDs and GIDs instead of names
	-s	List allocated blocks
	-lc	List ctime
	-lu	List atime
	--full-time	List full date and time
	-h	Human readable sizes (1K 243M 2G)
	--group-directories-first
	-S	Sort by size
	-X	Sort by extension
	-v	Sort by version
	-t	Sort by mtime
	-tc	Sort by ctime
	-tu	Sort by atime
	-r	Reverse sort order
	-w N	Format N columns wide
	--color[={always,never,auto}]	Control coloring

Describe what you expected to happen:
The correct long list

What version of fd are you using?
Installing fd (8.2.1-r0)

Which operating system / distribution are you on?
node:lts-alpine3.13
Executing busybox-1.32.1-r6.trigger

GitHub action

@sharkdp
Copy link
Owner

sharkdp commented Jun 28, 2021

Thank you for reporting this.

We already do some guesswork to call ls/gls with the correct arguments:

fd/src/main.rs

Lines 194 to 266 in f60b768

#[allow(unused)]
let gnu_ls = |command_name| {
vec![
command_name,
"-l", // long listing format
"--human-readable", // human readable file sizes
"--directory", // list directories themselves, not their contents
&color_arg,
]
};
let cmd: Vec<&str> = if cfg!(unix) {
if !cfg!(any(
target_os = "macos",
target_os = "dragonfly",
target_os = "freebsd",
target_os = "netbsd",
target_os = "openbsd"
)) {
// Assume ls is GNU ls
gnu_ls("ls")
} else {
// MacOS, DragonFlyBSD, FreeBSD
use std::process::{Command, Stdio};
// Use GNU ls, if available (support for --color=auto, better LS_COLORS support)
let gnu_ls_exists = Command::new("gls")
.arg("--version")
.stdout(Stdio::null())
.stderr(Stdio::null())
.status()
.is_ok();
if gnu_ls_exists {
gnu_ls("gls")
} else {
let mut cmd = vec![
"ls", // BSD version of ls
"-l", // long listing format
"-h", // '--human-readable' is not available, '-h' is
"-d", // '--directory' is not available, but '-d' is
];
if !cfg!(any(target_os = "netbsd", target_os = "openbsd")) && colored_output {
// -G is not available in NetBSD's and OpenBSD's ls
cmd.push("-G");
}
cmd
}
}
} else if cfg!(windows) {
use std::process::{Command, Stdio};
// Use GNU ls, if available
let gnu_ls_exists = Command::new("ls")
.arg("--version")
.stdout(Stdio::null())
.stderr(Stdio::null())
.status()
.is_ok();
if gnu_ls_exists {
gnu_ls("ls")
} else {
return Err(anyhow!(
"'fd --list-details' is not supported on Windows unless GNU 'ls' is installed."
));
}
} else {
return Err(anyhow!(
"'fd --list-details' is not supported on this platform."
));

... but we don't have a specialized option for the busybox version of ls. And I'm not sure if we could detect that somehow. We might need to parse the output of ls --version (or similar) in order to distinguish different implementations.

@peternewman
Copy link
Contributor Author

Yeah, alternatively, as a simpler option, are the short -l -h -d all standard on Linux? Would using them instead of the long names be an easier fix?

This run as mentioned was on a GitHub action, so fixing detection and ensuring no regression should be fairly easy at least.

@sharkdp
Copy link
Owner

sharkdp commented Aug 8, 2021

Using -l -h -d would work for GNU ls, yes. I like the --long-options for self-documenting purposes, but if this would really fix the problem on busybox, I'm okay to change it.

@sharkdp sharkdp added this to the fd 9 milestone Aug 8, 2021
@peternewman
Copy link
Contributor Author

I like the --long-options for self-documenting purposes

Makes sense.

but if this would really fix the problem on busybox, I'm okay to change it.

Well they were certainly reported in the help on the GitHub actions run as above, I wonder if they just don't bother with the complexity of long parsing in BusyBox.

@tmccombs
Copy link
Collaborator

tmccombs commented Aug 9, 2021

from the man page of busybox it does appear that -l -h -d would work. And according to https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/ the short options are posix standard, whereas the long options are not.

@peternewman
Copy link
Contributor Author

according to https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/ the short options are posix standard, whereas the long options are not.

Pedant, the -h option doesn't appear to be in the standard, although I'd imagine writing your own is more effort than it's worth.

@tmccombs
Copy link
Collaborator

Ah you're right. Are there any common implementations that are missing that?

@tavianator
Copy link
Collaborator

@tmccombs Not to my knowledge. It's present in at least coreutils, busybox, *BSD, and macOS.

@peternewman
Copy link
Contributor Author

@tmccombs Not to my knowledge. It's present in at least coreutils, busybox, *BSD, and macOS.

Yeah likewise here.

I guess you could stick a preemptive comment in the code to write your own -h function if some OS doesn't handle it in future.

sharkdp pushed a commit that referenced this issue Aug 10, 2021
Implement `--list-details` by using short options for `ls` to support more platforms (like BusyBox)

closes #783
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants