[
https://issues.apache.org/jira/browse/CB-12397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16480662#comment-16480662
]
ASF GitHub Bot commented on CB-12397:
-------------------------------------
brodybits closed pull request #8: CB-12397 fix .gitignore for plugins &
platforms (cordova-create part)
URL: https://github.com/apache/cordova-create/pull/8
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/index.js b/index.js
index 827f158..fabaefd 100644
--- a/index.js
+++ b/index.js
@@ -40,8 +40,8 @@ if (!global_config_path) {
* This will make the create internal events visible outside
* @param {EventEmitter} externalEventEmitter An EventEmitter instance that
will be used for
* logging purposes. If no EventEmitter provided, all events will be logged
to console
- * @return {EventEmitter}
- */
+ * @return {EventEmitter}
+ */
function setupEvents(externalEventEmitter) {
if (externalEventEmitter) {
// This will make the platform internal events visible outside
@@ -49,7 +49,7 @@ function setupEvents(externalEventEmitter) {
}
// There is no logger if external emitter is not present,
// so attach a console logger
- else {
+ else {
CordovaLogger.subscribe(events);
}
return events;
@@ -59,9 +59,9 @@ function setupEvents(externalEventEmitter) {
* Usage:
* @dir - directory where the project will be created. Required.
* @optionalId - app id. Required (but be "undefined")
- * @optionalName - app name. Required (but can be "undefined").
+ * @optionalName - app name. Required (but can be "undefined").
* @cfg - extra config to be saved in .cordova/config.json Required (but can
be "{}").
- * @extEvents - An EventEmitter instance that will be used for logging
purposes. Required (but can be "undefined").
+ * @extEvents - An EventEmitter instance that will be used for logging
purposes. Required (but can be "undefined").
**/
// Returns a promise.
module.exports = function(dir, optionalId, optionalName, cfg, extEvents) {
@@ -224,7 +224,7 @@ module.exports = function(dir, optionalId, optionalName,
cfg, extEvents) {
}).then(function(input_directory) {
var import_from_path = input_directory;
- //handle when input wants to specify sub-directory (specified in
index.js as "dirname" export);
+ //handle when input wants to specify sub-directory (specified in
index.js as "dirname" export);
var isSubDir = false;
try {
// Delete cached require incase one exists
@@ -254,7 +254,16 @@ module.exports = function(dir, optionalId, optionalName,
cfg, extEvents) {
// get stock hooks; used if template does not contain hooks
paths.hooks = path.join(require('cordova-app-hello-world').dirname,
'hooks');
-
+
+ // CB-12397 add .gitignore for plugins & platforms to app template
+ // get stock .npmignore; used if template does not contain .gitignore
+ // NOTE: This is part of a workaround for the npm .gitignore/.npmignore
+ // behavior discussed in:
+ // https://github.com/npm/npm/issues/1862
+ // https://github.com/npm/npm/issues/3763
+ // https://github.com/npm/npm/issues/7252
+ paths.npmignore =
path.join(require('cordova-app-hello-world').dirname, '.npmignore');
+
// ToDo: get stock package.json if template does not contain
package.json;
var dirAlreadyExisted = fs.existsSync(dir);
if (!dirAlreadyExisted) {
@@ -271,13 +280,23 @@ module.exports = function(dir, optionalId, optionalName,
cfg, extEvents) {
if (!!cfg.lib.www.link)
linkFromTemplate(import_from_path, dir);
- // If following were not copied/linked from template, copy from
stock app hello world
+ // If following were not copied/linked from template,
+ // copy from stock cordova-app-hello-world:
copyIfNotExists(paths.www, path.join(dir, 'www'));
copyIfNotExists(paths.hooks, path.join(dir, 'hooks'));
var configXmlExists = projectConfig(dir); //moves config to root
if in www
if (paths.configXml && !configXmlExists) {
shell.cp(paths.configXml, path.join(dir, 'config.xml'));
}
+
+ // CB-12397 add .gitignore for plugins & platforms to app template
+ // get stock .npmignore; used if template does not contain
.gitignore
+ // NOTE: This is part of a workaround for the npm
.gitignore/.npmignore
+ // behavior discussed in:
+ // https://github.com/npm/npm/issues/1862
+ // https://github.com/npm/npm/issues/3763
+ // https://github.com/npm/npm/issues/7252
+ shell.cp(paths.npmignore, path.join(dir, '.gitignore'));
} catch (e) {
if (!dirAlreadyExisted) {
shell.rm('-rf', dir);
@@ -326,7 +345,7 @@ module.exports = function(dir, optionalId, optionalName,
cfg, extEvents) {
if (cfg.name) conf.setName(cfg.name);
conf.setVersion('1.0.0');
conf.write();
- }
+ }
});
};
@@ -365,16 +384,16 @@ function copyTemplateFiles(templateDir, projectDir,
isSubDir) {
// Remove directories, and files that are unwanted
if (!isSubDir) {
var excludes = ['package.json', 'RELEASENOTES.md' , '.git',
'NOTICE', 'LICENSE', 'COPYRIGHT', '.npmignore'];
- templateFiles = templateFiles.filter( function (value) {
- return excludes.indexOf(value) < 0;
- });
+ templateFiles = templateFiles.filter( function (value) {
+ return excludes.indexOf(value) < 0;
+ });
}
// Copy each template file after filter
for (var i = 0; i < templateFiles.length; i++) {
copyPath = path.resolve(templateDir, templateFiles[i]);
shell.cp('-R', copyPath, projectDir);
}
- }
+ }
}
/**
@@ -406,9 +425,9 @@ function projectConfig(projectDir) {
/**
* Retrieve and read the .cordova/config file of a cordova project
- *
+ *
* @param {String} project directory
- * @return {JSON data} config file's contents
+ * @return {JSON data} config file's contents
*/
function dotCordovaConfig(project_root) {
var configPath = path.join(project_root, '.cordova', 'config.json');
@@ -422,7 +441,7 @@ function dotCordovaConfig(project_root) {
/**
* Write opts to .cordova/config.json
- *
+ *
* @param {String} project directory
* @param {Object} opts containing the additions to config.json
* @param {Boolean} autopersist option
@@ -443,13 +462,13 @@ function writeToConfigJson(project_root, opts,
autoPersist) {
}
return json;
} else {
- return json;
- }
+ return json;
+ }
}
/**
* Removes existing files and symlinks them if they exist.
- * Symlinks folders: www, merges, hooks
+ * Symlinks folders: www, merges, hooks
* Symlinks file: config.xml (but only if it exists outside of the www folder)
* If config.xml exists inside of template/www, COPY (not link) it to project/
* */
@@ -464,7 +483,7 @@ function writeToConfigJson(project_root, opts, autoPersist)
{
fs.symlinkSync(src, dst, type);
}
}
- }
+ }
// if template is a www dir
if (path.basename(templateDir) === 'www') {
linkSrc = path.resolve(templateDir);
diff --git a/spec/create.spec.js b/spec/create.spec.js
index f8db27e..8b5176e 100644
--- a/spec/create.spec.js
+++ b/spec/create.spec.js
@@ -97,7 +97,7 @@ describe('cordova create checks for valid-identifier',
function() {
})
.fin(done);
}, 60000);
-
+
it('should reject reserved words from end of id', function(done) {
create('projectPath', 'bob.class', 'appName', {}, events)
.fail(function(err) {
@@ -115,80 +115,73 @@ describe('create end-to-end', function() {
shell.mkdir('-p', tmpDir);
});
-
afterEach(function() {
process.chdir(path.join(__dirname, '..')); // Needed to rm the dir on
Windows.
shell.rm('-rf', tmpDir);
});
- function checkProject() {
+ function checkCommonArtifacts() {
// Check if top level dirs exist.
var dirs = ['hooks', 'platforms', 'plugins', 'www'];
dirs.forEach(function(d) {
expect(path.join(project, d)).toExist();
});
-
expect(path.join(project, 'hooks', 'README.md')).toExist();
+ // index.js and template subdir folder should not exist
+ // (inner files should be copied to the project folder)
+ expect(path.join(project, 'index.js')).not.toExist();
+ expect(path.join(project, 'template')).not.toExist();
+
// Check if www files exist.
expect(path.join(project, 'www', 'index.html')).toExist();
- // Check that config.xml was updated.
- var configXml = new ConfigParser(path.join(project, 'config.xml'));
- expect(configXml.packageName()).toEqual(appId);
+ // Check if config.xml exists.
+ expect(path.join(project, 'config.xml')).toExist();
- // TODO (kamrik): check somehow that we got the right config.xml from
the fixture and not some place else.
- // expect(configXml.name()).toEqual('TestBase');
+ // Check that config.xml does not exist inside of www
+ expect(path.join(project, 'www', 'config.xml')).not.toExist();
}
- function checkConfigXml() {
- // Check if top level dirs exist.
- var dirs = ['hooks', 'platforms', 'plugins', 'www'];
- dirs.forEach(function(d) {
- expect(path.join(project, d)).toExist();
- });
- expect(path.join(project, 'hooks', 'README.md')).toExist();
-
- //index.js and template subdir folder should not exist (inner files
should be copied to the project folder)
- expect(path.join(project, 'index.js')).not.toExist();
- expect(path.join(project, 'template')).not.toExist();
+ function checkCommonArtifactsAndConfigXml() {
+ checkCommonArtifacts();
- // Check if www files exist.
- expect(path.join(project, 'www', 'index.html')).toExist();
+ // Check that config.xml was updated.
var configXml = new ConfigParser(path.join(project, 'config.xml'));
expect(configXml.packageName()).toEqual(appId);
expect(configXml.version()).toEqual('1.0.0');
+ expect(configXml.name()).toEqual('TestBase');
+ }
- // Check that config.xml does not exist inside of www
- expect(path.join(project, 'www', 'config.xml')).not.toExist();
+ function checkArtifactsAndConfigXmlWithNoPackageFromWww() {
+ checkCommonArtifacts();
// Check that we got no package.json
expect(path.join(project, 'package.json')).not.toExist();
+ // Check that config.xml was updated.
+ var configXml = new ConfigParser(path.join(project, 'config.xml'));
+ expect(configXml.packageName()).toEqual(appId);
+ expect(configXml.version()).toEqual('1.0.0');
+ expect(configXml.name()).toEqual('TestBase');
// Check that we got the right config.xml from the template and not
stock
expect(configXml.description()).toEqual('this is the correct
config.xml');
}
- function checkSubDir() {
- // Check if top level dirs exist.
- var dirs = ['hooks', 'platforms', 'plugins', 'www'];
- dirs.forEach(function(d) {
- expect(path.join(project, d)).toExist();
- });
- expect(path.join(project, 'hooks', 'README.md')).toExist();
-
- //index.js and template subdir folder should not exist (inner files
should be copied to the project folder)
- expect(path.join(project, 'index.js')).not.toExist();
- expect(path.join(project, 'template')).not.toExist();
-
- // Check if config files exist.
- expect(path.join(project, 'www', 'index.html')).toExist();
+ function checkArtifactsAndConfigXmlWithPackageFromSubDir() {
+ checkCommonArtifacts();
// Check that config.xml was updated.
var configXml = new ConfigParser(path.join(project, 'config.xml'));
expect(configXml.packageName()).toEqual(appId);
expect(configXml.version()).toEqual('1.0.0');
+ expect(configXml.name()).toEqual('TestBase');
+
+ // Check that config.xml does not exist inside of www
+ expect(path.join(project, 'www', 'config.xml')).not.toExist();
+
delete require.cache[require.resolve(path.join(project,
'package.json'))];
+
// Check that we got package.json (the correct one)
var pkjson = require(path.join(project, 'package.json'));
// Pkjson.displayName should equal config's name.
@@ -206,8 +199,17 @@ describe('create end-to-end', function() {
// Create a real project with no template
// use default cordova-app-hello-world app
return create(project, appId, appName, {}, events)
- .then(checkProject)
.then(function() {
+ // GENERAL NOTE: since checkCommonArtifactsAndConfigXml() is
+ // called from an explicit function any failure traceback
+ // will show where it was triggered from.
+ checkCommonArtifactsAndConfigXml();
+ // Check that we got package.json
+ expect(path.join(project, 'package.json')).toExist();
+ // CB-12397 check .gitignore behavior
+ expect(path.join(project, '.gitignore')).toExist();
+ expect(path.join(project, '.npmignore')).toExist();
+ }).then(function() {
delete require.cache[require.resolve(path.join(project,
'package.json'))];
var pkgJson = require(path.join(project, 'package.json'));
//confirm default hello world app copies over package.json and it
matched appId
@@ -222,8 +224,14 @@ describe('create end-to-end', function() {
it('should successfully run with Git URL', function(done) {
// Create a real project with gitURL as template
return create(project, appId, appName, configGit, events)
- .then(checkProject)
- .fail(function(err) {
+ .then(function() {
+ checkCommonArtifactsAndConfigXml();
+ // Check that we got package.json
+ expect(path.join(project, 'package.json')).toExist();
+ // CB-12397 check .gitignore behavior
+ expect(path.join(project, '.gitignore')).toExist();
+ expect(path.join(project, '.npmignore')).toExist();
+ }).fail(function(err) {
console.log(err && err.stack);
expect(err).toBeUndefined();
})
@@ -236,8 +244,14 @@ describe('create end-to-end', function() {
// tests cache clearing of npm template
// uses phonegap-template-vue-f7-tabs
return create(project, appId, appName, configNPMold)
- .then(checkProject)
.then(function() {
+ checkCommonArtifactsAndConfigXml();
+ // Check that we got package.json
+ expect(path.join(project, 'package.json')).toExist();
+ // CB-12397 check .gitignore behavior
+ expect(path.join(project, '.gitignore')).toExist();
+ expect(path.join(project, '.npmignore')).toExist();
+ }).then(function() {
shell.rm('-rf', project);
delete require.cache[require.resolve(templatePkgJsonPath)];
var pkgJson = require(templatePkgJsonPath);
@@ -252,8 +266,8 @@ describe('create end-to-end', function() {
expect(err).toBeUndefined();
})
.fin(done);
- }, 60000);
-
+ }, 90000);
+
it('should successfully run with template not having a package.json at
toplevel', function(done) {
// Call cordova create with no args, should return help.
var config = {
@@ -267,8 +281,14 @@ describe('create end-to-end', function() {
};
// Create a real project
return create(project, appId, appName, config, events)
- .then(checkProject)
- .then(function(){
+ .then(function() {
+ checkCommonArtifactsAndConfigXml();
+ // Check that we got no package.json
+ expect(path.join(project, 'package.json')).not.toExist();
+ // CB-12397 check .gitignore behavior
+ expect(path.join(project, '.gitignore')).toExist();
+ expect(path.join(project, '.npmignore')).not.toExist();
+ }).then(function() {
// Check that we got the right config.xml
var configXml = new ConfigParser(path.join(project, 'config.xml'));
expect(configXml.description()).toEqual('this is the very correct
config.xml');
@@ -279,7 +299,7 @@ describe('create end-to-end', function() {
})
.fin(done);
}, 60000);
-
+
it('should successfully run with template having package.json and no sub
directory', function(done) {
// Call cordova create with no args, should return help.
var config = {
@@ -293,14 +313,20 @@ describe('create end-to-end', function() {
};
// Create a real project
return create(project, appId, appName, config, events)
- .then(checkProject)
- .fail(function(err) {
+ .then(function() {
+ checkCommonArtifactsAndConfigXml();
+ // Check that we got no package.json
+ expect(path.join(project, 'package.json')).not.toExist();
+ // CB-12397 check .gitignore behavior
+ expect(path.join(project, '.gitignore')).toExist();
+ expect(path.join(project, '.npmignore')).not.toExist();
+ }).fail(function(err) {
console.log(err && err.stack);
expect(err).toBeUndefined();
})
.fin(done);
}, 60000);
-
+
it('should successfully run with template having package.json, and
subdirectory, and no package.json in subdirectory', function(done) {
// Call cordova create with no args, should return help.
var config = {
@@ -315,21 +341,30 @@ describe('create end-to-end', function() {
// Create a real project
return create(project, appId, appName, config, events)
- .then(checkProject)
- .fail(function(err) {
+ .then(function() {
+ checkCommonArtifactsAndConfigXml();
+ // Check that we got no package.json
+ expect(path.join(project, 'package.json')).not.toExist();
+ // CB-12397 check .gitignore behavior
+ expect(path.join(project, '.gitignore')).toExist();
+ expect(path.join(project, '.npmignore')).not.toExist();
+ }).fail(function(err) {
console.log(err && err.stack);
expect(err).toBeUndefined();
})
.fin(done);
}, 60000);
-
it('should successfully run with template having package.json, and
subdirectory, and package.json in subdirectory', function(done) {
// Call cordova create with no args, should return help.
var config = configSubDirPkgJson;
return create(project, appId, appName, config, events)
- .then(checkSubDir)
- .fail(function(err) {
+ .then(function() {
+ checkArtifactsAndConfigXmlWithPackageFromSubDir();
+ // CB-12397 check .gitignore behavior
+ expect(path.join(project, '.gitignore')).toExist();
+ expect(path.join(project, '.npmignore')).not.toExist();
+ }).fail(function(err) {
console.log(err && err.stack);
expect(err).toBeUndefined();
})
@@ -341,8 +376,12 @@ describe('create end-to-end', function() {
var config = configConfigInWww;
// Create a real project
return create(project, appId, appName, config, events)
- .then(checkConfigXml)
- .fail(function(err) {
+ .then(function() {
+ checkArtifactsAndConfigXmlWithNoPackageFromWww();
+ // CB-12397 check .gitignore behavior
+ expect(path.join(project, '.gitignore')).toExist();
+ expect(path.join(project, '.npmignore')).not.toExist();
+ }).fail(function(err) {
console.log(err && err.stack);
expect(err).toBeUndefined();
})
@@ -360,8 +399,12 @@ describe('create end-to-end', function() {
}
};
return create(project, appId, appName, config, events)
- .then(checkConfigXml)
- .fail(function(err) {
+ .then(function() {
+ checkArtifactsAndConfigXmlWithNoPackageFromWww();
+ // CB-12397 check .gitignore behavior
+ expect(path.join(project, '.gitignore')).toExist();
+ expect(path.join(project, '.npmignore')).not.toExist();
+ }).fail(function(err) {
console.log(err && err.stack);
expect(err).toBeUndefined();
})
@@ -380,10 +423,10 @@ describe('create end-to-end', function() {
// Check if www files exist.
expect(path.join(project, 'www', 'index.html')).toExist();
-
- // Check www/config exists
+
+ // Check www/config exists
expect(path.join(project, 'www', 'config.xml')).toExist();
- // Check www/config.xml was not updated.
+ // Check www/config.xml was not updated.
var configXml = new ConfigParser(path.join(project, 'www',
'config.xml'));
expect(configXml.packageName()).toEqual('io.cordova.hellocordova');
expect(configXml.version()).toEqual('0.0.1');
@@ -393,18 +436,25 @@ describe('create end-to-end', function() {
expect(path.join(project, 'config.xml')).toExist();
configXml = new ConfigParser(path.join(project,
'config.xml'));
expect(configXml.description()).toEqual('this is the
correct config.xml');
- // Check project/config.xml was updated.
+ // Check project/config.xml was updated.
expect(configXml.packageName()).toEqual(appId);
expect(configXml.version()).toEqual('1.0.0');
// Check that we got no package.json
expect(path.join(project, 'package.json')).not.toExist();
- // Check that www is really a symlink,
- // and project/config.xml , hooks and merges are not
+ // Check that www is really a symlink, and
+ // project/config.xml, hooks, & merges are not.
expect(fs.lstatSync(path.join(project,
'www')).isSymbolicLink()).toBe(true);
expect(fs.lstatSync(path.join(project,
'hooks')).isSymbolicLink()).not.toBe(true);
expect(fs.lstatSync(path.join(project,
'config.xml')).isSymbolicLink()).not.toBe(true);
+
+ // Check that we got no package.json
+ expect(path.join(project, 'package.json')).not.toExist();
+
+ // CB-12397 FUTURE TBD: check .gitignore behavior
+ // expect(path.join(project, '.gitignore')).not.toExist();
+ expect(path.join(project, '.npmignore')).not.toExist();
}
var config = {
lib: {
@@ -440,7 +490,7 @@ describe('create end-to-end', function() {
expect(path.join(project, d)).toExist();
});
expect(path.join(project, 'hooks', 'README.md')).toExist();
-
+
//index.js and template subdir folder should not exist
(inner files should be copied to the project folder)
expect(path.join(project, 'index.js')).not.toExist();
expect(path.join(project, 'template')).not.toExist();
@@ -448,7 +498,7 @@ describe('create end-to-end', function() {
// Check if www files exist.
expect(path.join(project, 'www', 'index.html')).toExist();
- // Check that www, and config.xml is really a symlink
+ // Check that www, and config.xml are really symlinks.
expect(fs.lstatSync(path.join(project,
'www')).isSymbolicLink()).toBe(true);
expect(fs.lstatSync(path.join(project,
'config.xml')).isSymbolicLink()).toBe(true);
@@ -456,16 +506,23 @@ describe('create end-to-end', function() {
var configXml = new ConfigParser(path.join(project,
'config.xml'));
expect(configXml.packageName()).toEqual('io.cordova.hellocordova');
expect(configXml.version()).toEqual('0.0.1');
-
+
// Check that we got the right config.xml
expect(configXml.description()).toEqual('this is the
correct config.xml');
-
+
delete require.cache[require.resolve(path.join(project,
'package.json'))];
// Check that we got package.json (the correct one) and it
was changed
var pkjson = require(path.join(project, 'package.json'));
// Pkjson.name should equal config's id.
expect(pkjson.name).toEqual(appId.toLowerCase());
expect(pkjson.valid).toEqual('true');
+
+ // FUTURE TBD: Check that we got package.json or not
+ // expect(path.join(project,
'package.json')).not.toExist();
+
+ // CB-12397 FUTURE TBD: check .gitignore behavior
+ // expect(path.join(project, '.gitignore')).not.toExist();
+ expect(path.join(project, '.npmignore')).not.toExist();
}
var config = {
lib: {
@@ -515,6 +572,13 @@ describe('create end-to-end', function() {
expect(fs.lstatSync(path.join(project,
'hooks')).isSymbolicLink()).toBe(true);
expect(fs.lstatSync(path.join(project,
'merges')).isSymbolicLink()).toBe(true);
expect(fs.lstatSync(path.join(project,
'config.xml')).isSymbolicLink()).not.toBe(true);
+
+ // Check that we got no package.json
+ expect(path.join(project, 'package.json')).not.toExist();
+
+ // CB-12397 FUTURE TBD: check .gitignore behavior
+ // expect(path.join(project, '.gitignore')).not.toExist();
+ expect(path.join(project, '.npmignore')).not.toExist();
}
var config = {
@@ -543,5 +607,5 @@ describe('create end-to-end', function() {
.fin(done);
}, 60000);
- });
+ });
});
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> fix .gitignore for plugins & platforms in app template
> ------------------------------------------------------
>
> Key: CB-12397
> URL: https://issues.apache.org/jira/browse/CB-12397
> Project: Apache Cordova
> Issue Type: Improvement
> Components: cordova-app-hello-world, cordova-cli, cordova-create,
> cordova-lib
> Reporter: Chris Brody
> Priority: Major
> Labels: backlog, easy-fix
>
> Followup to CB-12008 (autosave by default in cordova@7): if a user creates an
> app using "cordova create" there should be a .gitignore file to exclude the
> plugins and platforms artifacts from git.
> I raise this since I have seen way too many apps with outdated plugins /
> platforms artifacts included in git.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
