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

sane headless operation #103

Open
n1vux opened this issue Sep 16, 2017 · 11 comments
Open

sane headless operation #103

n1vux opened this issue Sep 16, 2017 · 11 comments
Labels

Comments

@n1vux
Copy link
Contributor

n1vux commented Sep 16, 2017

re FAQ and --nofilter on beyondgrep/ack2#649 -- I agree this is an ack2 FAQ candidate.

but for ack3, I think we should make it work headless: explicit file(s) or directory(s) in @ARGV is sufficient user intent that they aren't intending to offer a pipe on STDIN (and they can say - if they intend to mix files and pipes), so I maintain least surprise is for ack in crontab to work same as on commandline. --nofilter doesn't sound like --work-in-crontab, hardly intuitive. At this point in 3.0, we aren't tied to not-breaking, so we can do the right thing. A better error and a better FAQ isn't much help when there's no TTY, it may be hard to see STDERR. Saying use grep for non-interactive isn't good enough. We're beyond-grep in multiple ways for non-interactive, non-highlight uses. ack --type=$whatever $badword $dir && handle_errors() is not unreasonable and may be much simpler than equivalent grep if $whatever is complex.

@petdance
Copy link
Collaborator

Isn't the crontab issue really a shell script issue?

Also, we have to remember Windows.

@petdance
Copy link
Collaborator

We need to look at the history of this issue before we do anything. This has come up annually or so since the dawn of ack.

@n1vux
Copy link
Contributor Author

n1vux commented Sep 17, 2017

Isn't the crontab issue really a shell script issue?

No.
Yes, there are plenty of Shell issues with crontabe whereby expecting what works in login shell to work won't (thin secure environment so no paths etc)
But an explicit path to one-file-ack and all needed libs in system perl (using built-in lib paths) will not hit those issues.
$path/ack --help and $path/ack version and echo foo | $path/ack foo all work in crontab just fine.
If those work, ack --txt badword /tmp && fixit.sh should work DWIM .

Also, we have to remember Windows.

Since neither crontab nor /dev/TTY appear on windows, I agree the fix to headless shouldn't break windows. if whatever code is forcing it to --filter mode despite explicit @argv suppling files/dirs beyond the options and mandatory pattern is to make WIN work, perhaps it needs to be protected by an IF (OS) {}

@petdance
Copy link
Collaborator

But an explicit path to one-file-ack

I'm talking about the TTY detection, not libs.

@n1vux
Copy link
Contributor Author

n1vux commented Sep 17, 2017

We need to look at the history of this issue before we do anything.

Agree, review of history is a good thing.
For 3.0 we have a golden opportunity to fix what is not fixable due to ack2.0 installed base compatibility.

This has come up annually or so since the dawn of ack.
Which is a clue that we have a DWIM failure. or a conflict of two DWIMs.

In the rest of the *nix infrastructure, if one expects to read STDIN pipe despite listing files on CLI, one includes - in the position desired to read STDIN

@n1vux
Copy link
Contributor Author

n1vux commented Sep 17, 2017

I'm talking about the TTY detection, not libs.

How is TTY detection a shell thing?

@n1vux
Copy link
Contributor Author

n1vux commented Sep 17, 2017

Why does nohup not provide equal breakage?

rm nohup.txt
/usr/bin/nohup ack --type=csv automotive CC*.csv &
sleep 5
cat nohup.txt

looks fine. Apparently nohup only fiddles FD [012] and SIGHUP, but doesn't change /dev/TTY

@n1vux
Copy link
Contributor Author

n1vux commented Sep 17, 2017

History.

Ack1 (all CLOSED)

  • 80 Bug: Piping to ack --count (fixed)
  • 84 --break must be set to 0 if we're piping (fixed in ack2)
  • 85 Add a - or --stdin option for compatibilty with grep (added in ack2, it says)
  • 90 Add a new --nofilter option added
  • 95 pipe detection on OS X fails (believed ok in ack2)
  • 106 wrong guess about filter mode when running inside Emacs (or Vim) moved to ack2 346, erroneously says fixed in.
  • 112 Return code does not work when piping data through ack (fixed in ack2)
  • 113 --pager option does not work when piping data through ack moved to ack2 # nnn
  • 158 ack erroneously goes into filter mode
    • Ignoring 1 argument on the command-line while acting as a filter. is "counter-intuitive (and not grep-compatible)," migrated Ack2 346 and fixed
  • 250 add --no-filter option to avoiding entering pipe mode (pull request) migrated to ack2 2

Ack2

  • 185 Undocumented option '--filter' is present bug docs task CLOSED
  • 187 Ack should only consider named pipes if they're provided directly on the command line CLOSED
  • 346 ack erroneously goes into filter modecombines ack1 158 & 106; moved to "Later" ; still OPEN
  • 475 No output when run in a loop. OPEN
  • 499 Error with implied --filter OPEN confusing behavior long thread.
  • 500 Arguments silently ignored in filter mode OPEN confusing behavior some discussion.
  • 501 - not treated as stdin OPEN
  • 502 remove --[no]filter option (pull rejected) discussion CLOSED
  • 551 Silent hanging on encountering a ./- file (inverse of 501) OPEN
  • 603 ack does not search stdin from terminal CLOSED
  • 614 Provide an option in ack to only parse files passed to it. requests -X like -x but respects --type`. OPEN
  • 649 ack outputs nothing using crontab in linux CLOSED with workaround --nofilter and follow-up :
  • 650 FAQ that --nofilter is required for headless use OPEN

Ack3

@petdance
Copy link
Collaborator

How is TTY detection a shell thing?

All I can tell you is that there have been problems with filter vs. direct detection in shell scripts. I don't know more specifics.

@n1vux
Copy link
Contributor Author

n1vux commented Sep 17, 2017

every other code I've seen that takes input from both listed files or STDIN, (a) does the filter DWIM based on if there are excess @argv or not, (b) accepted - as "read STDIN in turn here", (c) and ignored TTY.

Are we being too smart where others succeed by trusting the arguments?
Because we need to know if interactive to know whether to highlight DWIM or not?
Piped input does not mean not highlighting output.
Buffered-ness out STDOUT might be more appropriate clue to highlight DWIM?

@hugopeixoto
Copy link

I just ran into this issue in Gitlab CI. Assuming problems related to "interactiveness", I switched to grep and it worked.

Trying to understand why, I searched the man page for interactive and standard input/stdin, but couldn't find anything. I'm not sure if the --[no]filter option text is enough to highlight the issue.

I used strace and only saw calls to fstat(STDIN). Looking at the codebase, I traced it back to $is_filter_mode = -p STDIN; and its usage, which explains the problem: the first thing ack does is check if stdin is a pipe, and if so, it extracts an argv pattern, ignores argv paths and assumes that it is to filter text from stdin. This also causes echo foo | ack -f to also fail: if stdin is a pipe, it tries to extract an argv pattern that isn't there.

I agree that explicitly giving ARGV path to scan should trump STDIN. This doesn't need to affect ack's output, only its input (any TTY detection doesn't need to be touched). If changing this behavior is not an option, ack could at least:

  • emit a stderr warning if both stdin is a pipe (is_filter_mode) and there are argv files given;
  • have a note at the start of the man page stating something to the effect of "if both an stdin pipe and files are specified, files will be ignored"

For reference, here's a small test that shows the different behaviors using docker:

$ cat poop.sh
#!/usr/bin/env sh
apk add --update --no-cache ack file;
file /proc/self/fd/0
ack -f

$ BLABLA="--rm -w /src -v $(pwd):/src alpine:latest ./poop.sh"

$ docker run $BLABLA | grep proc
/proc/self/fd/0: symbolic link to /dev/null

$ docker run -i $BLABLA | grep proc
/proc/self/fd/0: symbolic link to pipe:[12640013]
ack: No regular expression found.

$ docker run -t $BLABLA | grep proc
/proc/self/fd/0: symbolic link to /dev/pts/0

$ docker run -it $BLABLA | grep proc
/proc/self/fd/0: symbolic link to /dev/pts/0

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

No branches or pull requests

3 participants