Error with implied --filter

[rdpate]

Given -f, when stdin is a pipe (or --filter makes it look like one), ack emits this error, even though it doesn't need to read stdin. I may have missed something trying to narrow this down. Test case:

$ ls  # starting in an empty directory
$ touch some-file
$ ack -f
some-file
$ ack -f <some-file
some-file
$ ack -f </dev/null
some-file
$ cat /dev/null | ack -f
ack: No regular expression found.
$ ack -f --filter
ack: No regular expression found.
$

(After diving through the code, it seems the test case can eliminate all except the first and last invocations of ack, but I'll leave it here in case it's useful to write a test.)

Expected behavior is what is shown, except for the last two error messages. I have ack 2.12 installed locally using the normal Ubuntu 14.04 package, but I get the same results with http://beyondgrep.com/ack-2.14-single-file.

The change below seems to do the right thing (edit: it doesn't), but I failed writing a test case (to be included in the repo tests), more because I don't know perl than anything else. This change does pass all the included tests and fixes the above error.

--- a/ack
+++ b/ack
@@ -1036,7 +1036,7 @@ sub main {
     }

     my $resources;
-    if ( $App::Ack::is_filter_mode && !$opt->{files_from} ) { # probably -x
+    if ( $App::Ack::is_filter_mode && !$opt->{files_from} && !$opt_f ) { # probably -x
         $resources    = App::Ack::Resources->from_stdin( $opt );
         $opt_regex = shift @ARGV if not defined $opt_regex;
         $opt_regex = $opt->{regex} = build_regex( $opt_regex, $opt );

[rdpate]

Also, I got around this bug by redirecting ack's stdin from /dev/null, since it doesn't need stdin and it just happened to be a pipe in what I was doing.

[hoelzro]

@rdpate Thanks for the bug report! This is definitely weird; can you give the "in the wild" example of how you encountered this? I'm wondering if we need to address the error message we print in this case.

[rdpate]

I was abusing ack to parse per-project .ackrc files and for its other file type ignoring, but I needed to include directories as well:

find -mindepth 1 -maxdepth 1 -type d ! -name '.*' | sed 's!^\./!!; s!$!/!' | sort - <(ack -n -f) | do-stuff

I used -n, but it doesn't seem to affect this bug. However, I'd have to parse .ackrc myself anyway to ignore directories listed there, and things just got too complicated.

I considered a feature request for ack to list directories it would search -- thoughts?

[hoelzro]

I would definitely expect ack to do the right thing in the case of <(ack -n -f) - it's interesting that it doesn't! It probably has to do with how <(...) works in the shell, which has been a point of pain in the past.

Regarding the list of directories to search feature, it sounds a lot like beyondgrep/ack2#310. Is that what you had in mind?

[petdance]

We need to tread carefully here. There have been problems and surprised expectations surrounding everything related to ack's handling of filter mode, STDIN detection, etc since the dawn of time, and we need to make sure we don't revert any previously fixed functionality.

[petdance]

Seems to me that perhaps the core problem here is that ack doesn't throw a warning if it's sent data on STDIN and called with -f. I can't see why one would legitimately want to do that.

[hoelzro]

@petdance That's the thing; in the "real world" use case above, I don't know why ack is getting data on STDIN, and to me it'd be the principle of least surprise if that usage worked. A warning would be a decent comprimise, though.

[petdance]

We already tell people "Hey, you're using -l and -A together, and that doesn't make sense." I think it would also make sense to say "Hey, you're using -f or -g in filter mode, and that doesn't make sense", too.

[rdpate]

Regarding the list of directories to search feature, it sounds a lot like beyondgrep/ack2#310. Is that what you had in mind?

No, I was thinking more like:

$ ls  # starting with empty directory
$ mkdir d d/e
$ touch a d/b
$ ack -f -d
a
d/
d/b
d/e/
$ ack -d  # show directories only
d/
d/e/

[petdance]

ack's -f only finds files. ack is only interested in files, not directories. It is not meant to be a general-purpose finder of things.

[rdpate]

Seems to me that perhaps the core problem here is that ack doesn't throw a warning if it's sent data on STDIN and called with -f. I can't see why one would legitimately want to do that.

I agree, you wouldn't. However, STDIN could just be a pipe that was used for something else entirely, and ack detects the pipe and turns on --filter. Specifically "$is_filter_mode = -p STDIN;" in Ack.pm. Example:

$ cat script
#!/bin/bash
while read; do
    :  # do something with input unrelated to later use of ack
done
ack -f
echo "---"
ack pattern script
$ echo a | ./script
ack: No regular expression found.
---
$ # not sure what happened to the last ack, it should have found one match
$ ack pattern script --with-filename --noheading
script:7:ack pattern script

The lack of error message and output from the last line of script leads me to believe the problem is not with -f and instead with using -p STDIN to turn on filter mode. Specifically, I'd expect cat /dev/null | ./script and ./script </dev/null to produce the same results, but they don't. I am motivated to learn more perl and dive more. :)

ack's -f only finds files. ack is only interested in files, not directories. It is not meant to be a general-purpose finder of things.

It's an abuse of ack's .ackrc files and built-in knowledge of what to ignore. I'm not submitting it as a feature request, but I wanted to clarify for @hoelzro. The end of the script file above shows the bug is unrelated to -f anyway, and should make a more general test case.

[hoelzro]

I think that if you do the following:

cat /dev/null | ack -f

You're asking for trouble. But if you're putting ack -f in a script, or you're doing <(ack -f), I don't think it's unreasonable to expect ack to work. That being said, I don't know if we really can differentiate between the various cases, so an error message might be the right way to proceed.

[rdpate]

In the process of exploring this issue, I found #270.

I think that if you do... You're asking for trouble.

I only did that to illustrate the difference when the exact same input comes from either a file or a pipe; I wouldn't do that otherwise. :) Cat is just a standin for taking input from a file or some utility that generates that file; tool >tempfile; ack pat <tempfile vs tool | ack pat. I still find it very weird that cat ... | ack pat and ack pat <... do completely different things.

Compared to grep which uses options (e.g. -r) and the presence/absence of arguments to determine whether to read stdin, ack assumes it should read stdin if it is a pipe, and only uses command-line arguments beyond the search pattern after checking that stdin is not a pipe. (edit: I was curious and tested ag, which behaves like ack testing -p STDIN when no file/dir arguments are specified and ignores stdin with those arguments. Not sure how much this should matter to ack, but it seems to support keeping existing behavior when there aren't file/dir arguments.)

But if you're putting ack -f in a script

It's not specific to -f, see the last line of script in this comment. Any use of ack in a script will break, as far as I can tell, if 1) that script's input may be either a pipe or anything not a pipe, 2) the script doesn't pipe into ack (which would force filter mode), and 3) --nofilter isn't given.

That being said, I don't know if we really can differentiate between the various cases, so an error message might be the right way to proceed.

Specifying both --filter and -f should be an error, but also -f should imply --nofilter (or otherwise disable the -p STDIN check).

I didn't mean to spend nearly this long on this today, but not in vain, besides the tiny bit of perl I learned. I see why ack breaks the unix symmetry of cat ...| vs <...; and tentatively agree with that decision. Thanks for ack, your time, and I'd consider this bug report closed, pending the previous paragraph about warning with -f.

[hoelzro]

Right, I wasn't trying to call you out! I suppose I should've phrased that "if one does...". I wish we could make ack work better with scripts, but until we find a solution, I think we'll have to stick with a warning.