tools: Copying contents of deps/npm to test-npm

[thefourtheye]

In Ubuntu, cp -r deps/npm/ test-npm/ was copying npm directory
under test-npm and the structure became test-npm/npm. But the test
requires the contents of deps/npm to be under test-npm. Using
deps/npm/. fixes it.

[Fishrock123]

LGTM. @mhdawson does this also fix your Ubuntu issue with test-npm.sh?

[targos]

LGTM. The fix is also needed on Fedora.

[silverwind]

Would * instead of . also work? That'd have been my approach. Also wonder why we didn't notice this earlier. Is it a difference between BSD and GNU cp?

[silverwind]

@thefourtheye what Ubuntu version is this? I can't seem to reproduce by running cp -r dir1/ dir2/ on 14.04 with cp (GNU coreutils) 8.21.

[targos]

@silverwind * also works on Fedora

[thefourtheye]

@silverwind Mine are as following

➜  git  lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 14.04.2 LTS
Release:        14.04
Codename:       trusty
➜  git  cp --version
cp (GNU coreutils) 8.21

I just tried again and I am able to reproduce the problem.

[silverwind]

@thefourtheye reproduced now too. It does only manifest when the target directory exists. Confirmed now on Ubuntu and Arch.

[thefourtheye]

I am really wondering why this . thingy has not been documented anywhere. I tried man and info.

[silverwind]

It's not shell expansion at least.

Also confirming this works on BSD cp, so LGTM.

[jbergstroem]

Can't we just omit a slash?

$ cp -r deps/npm test-npm/

Edit, output from Linux iojs-build-ubuntu1404-64-1 3.13.0-37-generic:

$ mkdir -p deps/npm/foo test-npm
$ cp -r deps/npm test-npm/
$ ls -R test-npm
test-npm:
npm

test-npm/npm:
foo

test-npm/npm/foo:

[silverwind]

@jbergstroem that won't work, I think we want test-npm to be the npm package root, without an npm subfolder.

[jbergstroem]

@silverwind hm, I thought that was what we wanted since we created test-npm just above. If not, we should remove mkdir and just avoid the trailing slash:

$ cp -r deps/npm test-npm

Edit: You're right. Looks like the script assumes we're in that folder. Then I'd suggest above -- remove the mkdir and just use cp to create it.

[silverwind]

Yeah, that mkdir is unnecessary. @thefourtheye care to change the PR to @jbergstroem's suggestion?

[thefourtheye]

@silverwind @jbergstroem Updated the PR. Can you guys review this now?

[silverwind]

LGTM

[jbergstroem]

LGTM

[silverwind]

Thanks, landed in 1baba05

[bnoordhuis]

@thefourtheye Would it be possible for you to sign your commits with your real name? You show up as 'thefourtheye' in Author fields now but I see you have a couple of Reviewed-By tags under your full name.

[thefourtheye]

@bnoordhuis Oh sorry. I changed it in my local git config but the last commits were made directly from github. Do you want me to change them? If so, please let me know how I can do that.

[bnoordhuis]

They're in the history now so that's that. I speculate that the GH editor uses the name and email that are visible in your profile but I don't know for sure, I never use it.

Can I suggest you just make your changes from the command line? Almost every change requires that you run make test (and, implicitly, make lint) anyway.

[thefourtheye]

@bnoordhuis Sure, next time onwards I ll do it from a local branch and sorry about this time. But before submitting the PR, I locally tested the changes and since the change was very small I directly edited it.

[bnoordhuis]

No problem, it's not a big thing, just a small incongruity I noticed.