5

In the directories ~/temp/a/foo/ and ~/temp/b/foo foo/ I have some files named bar1, bar2, bar bar1, bar bar2, etc.

I am trying to write a line of Bash that copies all these files in a directory containing "foo" as last part of the name to the folder above the respective "foo" folder.

As long as there are no spaces in the file names, this is an easy task, but the two following commands fail when dealing with the foo foo directory:

for dir in `find . -type d -name '*foo'` ; do cp $dir/* "$(echo $dir|sed 's_foo__g')" ; done

(The cp command fails to see the last foo of "foo foo" as part of the same directory name.)

for dir in `find . -type d -name '*foo'` ; do cp "$dir/*" "$(echo $dir|sed 's_foo__g')" ; done

("$dir/*" is not expanded.)

Attempts like replacing $dir/* with "$(echo $dir/*)" have been even less successful.

Is there an easy way to expand $dir/* so that cp understands?

uvett
  • 69
  • 7

3 Answers3

4

Not only is a for loop wrong -- sed is also not the right tool for this job.

while IFS= read -r -d '' dir; do
  cp "$dir" "${dir/foo/}"
done < <(find . -type d -name '*foo' -print0)

Using -print0 on the find (and IFS= read -r -d '') ensures that filenames with newlines won't mess you up.

Using the < <(...) construct ensures that if the inside of your loop sets variables, changes directory state, or does similar things, those changes will survive (the right-hand side of a pipeline is in a subshell in bash, so piping into a while loop would mean that any changes to the shell's state made inside that while loop would be discarded on its exit otherwise).

Using ${dir/foo/} is vastly more efficient than invoking sed, as it does the string replacement internal to bash.

Charles Duffy
  • 280,126
  • 43
  • 390
  • 441
  • Are you sure the `IFS=` is needed there? Great post btw! – janos Mar 16 '13 at 23:41
  • Despite janos's suggestion to accept this answer as a solution, it turns out that the code does not actually move any files. Bash 4.2.24 complains that cp cannot stat ''. I am not 100% sure what the line `cp "$dir" "${dir/foo/}"` does. Does it really attempt to copy files to an existing directory above the `foo` directory? – uvett Mar 17 '13 at 00:13
  • @uvett Dan pegged the error. If that doesn't fix it, be sure your file starts with `#!/bin/bash`; this code won't work with `#!/bin/sh`. – Charles Duffy Mar 17 '13 at 05:45
  • @janos The `IFS=` prevents trailing whitespace from being stripped. (This could also be avoided by not specifying `dir` and allowing the content read to be assigned to `$REPLY`). – Charles Duffy Mar 17 '13 at 05:46
  • Sorry, no luck. As long as I use `cp -r` and the `foo` directory contains characters preceding `foo`, I am able to move files with these lines, but in this case, the line `cp -r "$dir" "${dir/foo/}"` (obviously) creates a new directory based on the old directory name, subtracting the `foo` part. With directories named only `foo`, the command fails. My first thoughts concerning how to solve this are exactly the same as the ones that made me create this thread: Replacing Charles's `cp "$dir" "${dir/foo/}"` with `cp "$dir/*"` `"${dir/foo}"` (useless because the quotes prevent asterisk expansion). – uvett Mar 17 '13 at 09:53
  • @uvett Oh. No need for string substitution at all in that case. Inside the `find`-driven `while` loop: `cp -r "$dir"/* "$dir/.."` – Charles Duffy Mar 17 '13 at 17:45
  • @ Charles Duffy Very sweet and robust, thanks again. `cp -r` turned out to be unneccessary for the problem I described in my question. Not sure if the code in this answer should be edited, since the string substitution part could (should) be left out - at the same time, it is the subject of some interesting comments. – uvett Mar 17 '13 at 19:58
2

The problem here is not with cp, but with for, because by default it splits the output of your subshell by words, not by directory names.

A lazy workaround is to use while instead and process the list of directories line by line like this:

find . -type d -name '*foo' | while read dir; do cp "$dir"/* "$(echo $dir | sed 's_foo__g')" ; done

This should fix your problem with spaces, but this is by no means a foolproof solution.

See Charles Duffy's answer for a more accurate solution.

janos
  • 120,954
  • 29
  • 226
  • 236
  • Almost right, but you really should NUL-delimit the contents of the pipeline -- otherwise, a maliciously crafted filename containing a newline can cause any arbitrary file to be copied. – Charles Duffy Mar 16 '13 at 23:35
  • ...by the way, it's also not accurate that `dir` will never contain spaces as an artifact of how `for` works; word-splitting is _not_ part of for's semantics, and if `IFS` is set to not contain whitespace, string-split output _can_ contain whitespace. That said, relying on string-splitting is wrong for other reasons (for instance, because glob expansion happens at the same time). – Charles Duffy Mar 16 '13 at 23:40
  • Thanks, this seems to work for all my real-world file-copying problems. That it actually fails on my simplified example, is a funny detail. (sed deletes both `foo`s in the `foo foo` folder name). – uvett Mar 16 '13 at 23:45
  • @CharlesDuffy "almost right"... is a bit far from the truth eh :-) But that's exactly why I added the "by no means foolproof" bit. Thanks for the tips, I'll rephrase what I wrote about `for`. – janos Mar 16 '13 at 23:47
  • @uvett I urge you to accept Charles Duffy's answer instead of mine. It's more accurate, better explained and it takes care of all corner cases. – janos Mar 16 '13 at 23:49
0

Rename ~/temp/b/foo foo/ to something without spaces, e.g. ~/temp/b/foo_foo/ and do what you were trying to do again. After that you can rename it back to the original, with space, if you really have to. BTW. myself I never use file names containing spaces, simply to avoid complications like the one you are facing now.

piokuc
  • 25,594
  • 11
  • 72
  • 102
  • I attempted to simplfy a problem that concerns dozens of folders and hundreds of files with ugly names. Renaming them seems like a sub-optimal option, especially since I would have to restore the folder names later. – uvett Mar 16 '13 at 23:18