[TIMOB-14386] Fixed bug in copyFileSync() when copying a file to a directory.

CHANGELOG.md

@@ -25,6 +25,7 @@
  * Added new string utility functions wrap() and renderColumns()
  * Internationalized strings in the time library
  * Better/cleaner Titanium module and CLI plugin detection
+ * Fixed bug in copyFileSync() when copying a file to a directory [TIMOB-14386]
 
 0.1.30
 -------------------

lib/fs.js

@@ -105,7 +105,7 @@ exports.visitDirsSync = function (dir, visitor) {
 };
 
 exports.touch = function (file) {
-	if (exports.exists(file)) {
+	if (fs.existsSync(file)) {
 		fs.utimesSync(file, new Date, new Date);
 	} else {
 		fs.writeFileSync(file, '');
@@ -121,10 +121,10 @@ exports.isDirWritable = function (directory) {
 	var result = false,
 		dir = exports.resolvePath(directory),
 		tmpFile = path.join(dir, 'tmp' + Math.round(Math.random() * 1e12));
-	if (exports.exists(dir)) {
+	if (fs.existsSync(dir)) {
 		try {
 			exports.touch(tmpFile);
-			result = exports.exists(tmpFile);
+			result = fs.existsSync(tmpFile);
 			fs.unlinkSync(tmpFile);
 		} catch (e) {}
 	}
@@ -135,14 +135,12 @@ exports.isDirWritable = function (directory) {
 // If the destination is a existing file, it uses the destination file's name. If the destination does NOT
 // exist, it assumes the dest is a path to a file.
 exports.copyFileSync = function (src, dest, opts) {
-	var destFilename = path.basename(src);
-	if (!exports.exists(dest) || !fs.lstatSync(dest).isDirectory()) {
-		destFilename = path.basename(dest);
-		dest = path.dirname(dest);
+	if (fs.existsSync(dest) && fs.statSync(dest).isDirectory()) {
+		dest = path.join(dest, path.basename(src));
 	}
-	wrench.mkdirSyncRecursive(dest);
-	dest = path.join(dest, destFilename);
-	opts && opts.logger && opts.logger(__('Copying %s => %s', src.cyan, dest.cyan));
+	opts && typeof opts.logger == 'function' && opts.logger(__('Copying %s => %s', src.cyan, dest.cyan));
+	var p = path.dirname(dest);
+	fs.existsSync(p) || wrench.mkdirSyncRecursive(p);
 	fs.writeFileSync(dest, fs.readFileSync(src));
 };
 
@@ -181,7 +179,7 @@ exports.copyDirSyncRecursive = function(sourceDir, newDirLocation, opts) {
 		} else {
 			if (opts.ignoreFiles && opts.ignoreFiles.indexOf(files[i]) != -1) continue;
 
-			if (exports.exists(destFile)) {
+			if (fs.existsSync(destFile)) {
 				fs.unlinkSync(destFile);
 			}
 
@@ -285,7 +283,7 @@ exports.nonDestructiveCopyDirSyncRecursive = function(sourceDir, newDirLocation,
 				destFilePath = path.join(newDirLocation, dir, file)
 			if (fs.statSync(sourceFilePath).isDirectory()) {
 				pathStack.push(path.join(dir, file));
-			} else if (!exports.exists(destFilePath)) {
+			} else if (!fs.existsSync(destFilePath)) {
 				if (!ignoreHiddenFiles || path.basename(sourceFilePath).charAt(0) !== '.') {
 					exports.copyFileSync(sourceFilePath, destFilePath, opts);
 					numCopied++;

tests/resources/testfile.txt

@@ -0,0 +1,41 @@
+Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce pulvinar placerat
+magna non vulputate. Aliquam erat volutpat. Donec vitae velit vitae elit porta
+pellentesque quis sed ante. Aliquam erat volutpat. Mauris gravida est turpis,
+sit amet pharetra nibh rhoncus non. Nam faucibus dolor turpis, ac pellentesque
+elit mollis sed. Quisque ac mollis nunc, at accumsan sapien. Morbi magna eros,
+pharetra ac dictum hendrerit, tristique sed metus. In lobortis eget diam commodo
+viverra.
+
+In ultricies ipsum eget quam porttitor, in pulvinar neque viverra. Maecenas
+lacus mauris, condimentum a quam at, interdum porta ante. Pellentesque luctus
+facilisis velit, eget gravida neque lobortis a. Ut in imperdiet neque, sit amet
+molestie felis. Phasellus lacus neque, bibendum id facilisis sollicitudin,
+molestie sed felis. Fusce dignissim, nulla non laoreet dictum, dui ante
+pellentesque ipsum, quis euismod lectus nisl quis magna. Vestibulum quam libero,
+rhoncus sed orci a, feugiat ornare metus. Maecenas eros risus, pretium sit amet
+turpis posuere, dictum molestie lectus.
+
+In in malesuada diam, a molestie lacus. In in risus sed ligula laoreet
+facilisis. Nullam facilisis ut ligula vel egestas. Mauris facilisis tortor eget
+enim volutpat, non pulvinar tellus aliquam. Quisque at accumsan velit. Nunc leo
+leo, adipiscing et mi et, tempor dictum erat. Aliquam nec blandit elit, vel
+facilisis est. In dignissim tristique tortor eget pharetra. Ut cursus elit id
+eros pretium, eu dignissim mauris fermentum. Sed tincidunt eget lorem in mattis.
+
+Etiam egestas diam at est blandit vestibulum. Pellentesque a hendrerit eros,
+eget lacinia erat. Nullam molestie faucibus ligula, at sodales odio mollis sed.
+Aenean consequat est eu sapien condimentum tincidunt. Curabitur ac imperdiet
+leo. Aliquam ut magna non diam vulputate pretium. Pellentesque semper, nisl ac
+consequat molestie, metus velit tempus dui, sed porta massa nunc et ligula. Cras
+mauris urna, tempor sed mi sit amet, tempor tristique purus. Morbi sed lacus
+leo. Aenean vulputate, arcu vel tincidunt rhoncus, erat tortor eleifend diam,
+sit amet placerat felis quam ac neque. Pellentesque habitant morbi tristique
+senectus et netus et malesuada fames ac turpis egestas. Nunc bibendum, nunc quis
+sodales ultrices, dui sem mollis tortor, sed lobortis nibh sapien quis dolor.
+
+Nunc aliquam lacus tortor, sed porttitor est tempor quis. Sed consectetur sem
+erat, at lacinia mauris dictum scelerisque. In id porta ligula. Morbi quis odio
+et quam egestas dignissim et sagittis turpis. Pellentesque quis adipiscing urna,
+in ultricies justo. Pellentesque habitant morbi tristique senectus et netus et
+malesuada fames ac turpis egestas. In ullamcorper malesuada pulvinar. Maecenas
+pretium in lacus vitae faucibus.

tests/test-fs.js

@@ -0,0 +1,78 @@
+/**
+ * node-appc - Appcelerator Common Library for Node.js
+ * Copyright (c) 2009-2013 by Appcelerator, Inc. All Rights Reserved.
+ * Licensed under the terms of the Apache Public License
+ * Please see the LICENSE included with this distribution for details.
+ */
+
+var appc = require('../index'),
+	assert = require('assert'),
+	fs = require('fs'),
+	path = require('path'),
+	temp = require('temp'),
+	wrench = require('wrench');
+
+function MockLogger() {
+	this.buffer = '';
+	this.debug = function (s) { this.buffer += s + '\n'; };
+	this.info = function (s) { this.buffer += s + '\n'; };
+	this.warn = function (s) { this.buffer += s + '\n'; };
+	this.error = function (s) { this.buffer += s + '\n'; };
+}
+
+describe('fs', function () {
+	it('namespace exists', function () {
+		appc.should.have.property('fs');
+		appc.fs.should.be.a('object');
+	});
+
+	describe('#copyFileSync()', function () {
+		it('copy file to file with existing directory', function () {
+			var logger = new MockLogger,
+				src = path.join(__dirname, 'resources', 'testfile.txt'),
+				dest = path.join(temp.mkdirSync(), 'testfile.txt');
+			appc.fs.copyFileSync(src, dest, { logger: logger.info.bind(logger) });
+			logger.buffer.stripColors.should.equal('Copying ' + src + ' => ' + dest + '\n');
+			assert(fs.existsSync(dest), 'Destination file does not exist');
+		});
+
+		it('copy file to file with non-existent directory', function () {
+			var logger = new MockLogger,
+				src = path.join(__dirname, 'resources', 'testfile.txt'),
+				dest = path.join(temp.mkdirSync(), 'test', 'testfile.txt');
+			appc.fs.copyFileSync(src, dest, { logger: logger.info.bind(logger) });
+			logger.buffer.stripColors.should.equal('Copying ' + src + ' => ' + dest + '\n');
+			assert(fs.existsSync(dest), 'Destination file does not exist');
+		});
+
+		it('copy file to /tmp', function () {
+			var logger = new MockLogger,
+				src = path.join(__dirname, 'resources', 'testfile.txt'),
+				dest = '/tmp';
+			fs.existsSync(dest) || wrench.mkdirSyncRecursive(dest);
+			appc.fs.copyFileSync(src, dest, { logger: logger.info.bind(logger) });
+			logger.buffer.stripColors.should.equal('Copying ' + src + ' => ' + dest + path.sep + 'testfile.txt\n');
+			assert(fs.existsSync(path.join(dest, 'testfile.txt')), 'Destination file does not exist');
+		});
+
+		it('copy file to existing directory', function () {
+			var logger = new MockLogger,
+				src = path.join(__dirname, 'resources', 'testfile.txt'),
+				dest = temp.mkdirSync();
+			appc.fs.copyFileSync(src, dest, { logger: logger.info.bind(logger) });
+			logger.buffer.stripColors.should.equal('Copying ' + src + ' => ' + dest + path.sep + 'testfile.txt\n');
+			assert(fs.existsSync(path.join(dest, 'testfile.txt')), 'Destination file does not exist');
+		});
+
+		it('copy file to non-existent directory', function () {
+			var logger = new MockLogger,
+				src = path.join(__dirname, 'resources', 'testfile.txt'),
+				dest = path.join(temp.mkdirSync(), 'test');
+			// since dest does not exist, it doesn't know it's a directory and the dest
+			// file will be named "test" and not "testfile.txt"
+			appc.fs.copyFileSync(src, dest, { logger: logger.info.bind(logger) });
+			logger.buffer.stripColors.should.equal('Copying ' + src + ' => ' + dest + '\n');
+			assert(fs.existsSync(dest), 'Destination file does not exist');
+		});
+	});
+});