capistrano-plugins / capistrano-faster-assets

Skips asset compilation if none of the assets were changed since last release.
MIT License
187 stars 38 forks source link

add T option #14

Closed kenjiszk closed 8 years ago

kenjiszk commented 8 years ago

Add T option to cp

In the target directory existing case, cp command doesn't work as expected.

$ mkdir -p source/assets
$ mkdir -p target/assets
$ touch source/assets/sample.txt
$ cp -r source/assets target/assets
$ tree target
target
`-- assets
    `-- assets
        `-- sample.txt

2 directories, 1 file

then, add T option, it works.

$ rm -fr target/assets/assets 
$ cp -Tr source/assets target/assets
$ tree target
target
`-- assets
    `-- sample.txt

1 directory, 1 file

It also works when target directory is not exists.

$ mkdir -p source/assets
$ mkdir target
$ touch source/assets/sample.txt
$ cp -Tr source/assets target/assets
$ tree target 
target
`-- assets
    `-- sample.txt

1 directory, 1 file
rhomeister commented 8 years ago

I'm not familiar with the -T option. man cp says treat DEST as a normal file. Is this really what we want?

kenjiszk commented 8 years ago

Yes, if cp treats DEST as a directory, copy result becomes target/assets/assets/sample.txt

See also https://www.gnu.org/software/coreutils/manual/html_node/Target-directory.html

rhomeister commented 8 years ago

@kenjiszk, @elik-ru disagrees with this solution (see https://github.com/capistrano-plugins/capistrano-faster-assets/pull/16). Could you please provide arguments why this commit should remain? If not, I will revert this PR.

kenjiszk commented 8 years ago

@rhomeister Please revert this PR. I think big issue is this( @elik-ru thank you for comment ). -T key exists only in Linux, not in FreeBSD, MacOS and may be some other systems

I will think about another solution better than 'add T'.

rhomeister commented 8 years ago

OK, thanks for your input. I've reverted the PR.

silva96 commented 7 years ago

I back this, is the only way this gem can work for me.