opened
mmstick
username_0
With this change, the `--exec` flag will be fully functional with a syntax similar to GNU Parallel. The argument that follows the `--exec` flag will be used as the command template, and each path discovered is distributed across a job pool for command generation & execution.

The README has been updated to demonstrate the new feature.

Closes #84
Implement the --exec flag
issue
created
mmstick
username_0
I'm guessing Rust 1.16 is a required target? Will need to do a few minor changes to get it building on that one.
comment
created
sharkdp
username_1
I was actually thinking about increasing this for some time. What would be required for this PR?
comment
created
mmstick
username_0
Nothing's truly required. It's largely just syntax enhancements that have come about to make Rust more user-friendly, like not requiring that match arm scopes have commas, and `eprintln`.
comment
created
sharkdp
username_1
Sorry.. I meant: which Rust version would be required as new minimum version? 1.19?
comment
created
sharkdp
username_1
src/main src/main.rs
```

The last one looks like '{}' is appended in addition to '{.}'. Is this intended?
comment
created
mmstick
username_0
Yes, that is the behavior that's used in GNU Parallel. If no placeholder token is provided, it is assumed that the user wants to append the placeholder at the end of the command. So there's no need to explicitly state where you want your input, if you want the input at the end.
comment
created
mmstick
username_0
Older builds should be working now, and fixed the double placement issue.
comment
created
mmstick
username_0
Rust 1.19 will be a requirement.
comment
created
sharkdp
username_1
Okay. We should update `.travis.yml` to reflect that.
comment
created
mmstick
username_0
Done.
comment
created
sharkdp
username_1
tmp/foo
```
comment
created
mmstick
username_0
Do you want outputs to be printed serially, in the order that inputs are processed?
comment
created
sharkdp
username_1
I don't think that's needed since there is no well-defined order in which we traverse the file-system (due to there being several threads walking the filesystem).
comment
created
mmstick
username_0
I can easily go ahead and fix the absolute path part, if you want me to.
comment
created
sharkdp
username_1
That would be great.
comment
created
mmstick
username_0
Absolute paths are working now.

```
username_0:~/S/fd# target/debug/fd -a --exec 'echo {}'
/home/username_0/Sources/fd/Cargo.lock
/home/username_0/Sources/fd/LICENSE
/home/username_0/Sources/fd/Cargo.toml
/home/username_0/Sources/fd/CONTRIBUTING.md
/home/username_0/Sources/fd/README.md
/home/username_0/Sources/fd/appveyor.yml
/home/username_0/Sources/fd/src
/home/username_0/Sources/fd/tests/tests.rs
/home/username_0/Sources/fd/tests
/home/username_0/Sources/fd/src/app.rs
/home/username_0/Sources/fd/src/internal.rs
/home/username_0/Sources/fd/src/exec
/home/username_0/Sources/fd/src/main.rs
/home/username_0/Sources/fd/src/fshelper
/home/username_0/Sources/fd/src/lscolors
/home/username_0/Sources/fd/src/walk.rs
/home/username_0/Sources/fd/src/exec/paths.rs
/home/username_0/Sources/fd/src/output.rs
/home/username_0/Sources/fd/src/fshelper/mod.rs
/home/username_0/Sources/fd/src/exec/mod.rs
/home/username_0/Sources/fd/build.rs
/home/username_0/Sources/fd/src/exec/token.rs
/home/username_0/Sources/fd/src/exec/job.rs
/home/username_0/Sources/fd/src/lscolors/mod.rs
/home/username_0/Sources/fd/src/exec/ticket.rs
/home/username_0/Sources/fd/tests/testenv/mod.rs
/home/username_0/Sources/fd/tests/testenv
```
comment
created
mmstick
username_0
I could also implement grouping of outputs, if you want outputs to be grouped by job (only print the stdout/stderr of one job at a time).
comment
created
mmstick
username_0
Implemented a mechanism for grouping outputs. It uses `std::process::Output` to act as a buffer for piped outputs, and a `Arc<Mutex<()>>` to serve as a lock for ensuring that only thread has write access at a given time.

It's the easiest way to implement this in a cross-platform manner. Only downside is that it has to perform heap allocations to buffer outputs, and will only print outputs when a job is complete.
comment
created
mmstick
username_0
Am working on an optimized variant of grouping for *nix systems.
comment
created
mmstick
username_0
Alright, *nix-specific specialization have been implemented. Shouldn't be any heap allocations on that end, beyond allowing the kernel to handle all buffering of outputs.
comment
created
mmstick
username_0
Performance counter stats for 'fd -x echo {} .rs' (100 runs):

         75.216770      task-clock:u (msec)       #    2.363 CPUs utilized            ( +-  1.08% )
                 0      context-switches:u        #    0.000 K/sec                  
                 0      cpu-migrations:u          #    0.000 K/sec                  
             3,364      page-faults:u             #    0.045 M/sec                    ( +-  0.25% )
        42,427,488      cycles:u                  #    0.564 GHz                      ( +-  3.92% )  (87.17%)
         9,996,856      stalled-cycles-frontend:u #   23.56% frontend cycles idle     ( +-  0.73% )  (89.05%)
         8,013,443      stalled-cycles-backend:u  #   18.89% backend cycles idle      ( +-  1.48% )  (32.36%)
        42,041,689      instructions:u            #    0.99  insn per cycle         
                                                  #    0.24  stalled cycles per insn  ( +-  8.39% )  (59.72%)
        10,112,065      branches:u                #  134.439 M/sec                    ( +-  8.44% )  (72.51%)
           124,204      branch-misses:u           #    1.23% of all branches          ( +-  0.89% )  (85.12%)

       0.031826863 seconds time elapsed                                          ( +-  0.85% )
```
comment
created
mmstick
username_0
So it seems the reason that `find` has an advantage is because it acts as it's own shell. It doesn't use any shell at all -- executes them natively. That would make sense.
comment
created
sharkdp
username_1
I like that this spawns a shell, makes it more powerful.

Do you still want to work on this or do you think it's ready to be integrated?

I'm in favor of merging this in early, but unfortunately there are a few conflicts now due to #113.
comment
created
mmstick
username_0
I fixed the conflicts. It should be ready to merge now, and fully feature-complete.
comment
created
sharkdp
username_1
Possibly naive question: There are quite a lot of `unsafe` blocks now. Are these strictly necessary or due to optimizations?
comment
created
mmstick
username_0
All the `unsafe` blocks are just within the `then_execute` method for *nix systems, because any call to a function in `libc` (`pipe`/`dup2`/`close`) requires you to use the `unsafe` keyword. It's not actually unsafe -- they are just kernel system calls to create and manage anonymous file descriptors.

The worst that'd happen is that if the programmer didn't write that section correctly (ie: remove some of those close calls), the piped file descriptors would cause each thread to hang at `io::copy()`, which is easily debugged (if, for whatever reason, someone decides to modify that code).
comment
created
sharkdp
username_1
Thank you for the clarification.

The `unsafe`-block in `main.rs` is not FFI-specific though, right?
comment
created
mmstick
username_0
The unsafe block in `walk.rs` is a workaround to get the reference to the `TokenizedCommand` to be shared across threads, by converting the reference into an Arc. The borrow checker, at this time, just isn't smart enough to realize that the data lives well beyond the `join()` calls for the thread handles of each worker, and that the original data is never mutated during that timeframe.
comment
created
sharkdp
username_1
Doen't quite seem to work on Windows (see inline comment). I think we should add a few integration tests after all (see `tests/tests.rs`). Maybe simply in the style of `fd -x echo` ==`fd` where. What do you think?
comment
created
mmstick
username_0
I'll look into writing integration tests when I get back home.
comment
created
sharkdp
username_1
Thank you very much for all of this! I'm going to merge this now and we can add the integration tests later.
comment