Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/rules multi sources #340

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Feature/rules multi sources #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
66bce9d
checkpoint
Jun 27, 2017
a2ee3e2
bugfix
Jun 27, 2017
032ad46
tests working
Jun 27, 2017
848ea31
whitespace
Jun 28, 2017
afe002d
more whitespace
Jun 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 63 additions & 15 deletions lib/rule.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,43 +236,84 @@ Rule.prototype = new (function () {
, source
, action
, opts
, prereqs
, parts
, valid
, src
, tNs
, createdTask
, name = Task.getBaseTaskName(fullName)
, nsPath = Task.getBaseNamespacePath(fullName)
, ns = this.ns.resolveNamespace(nsPath);
, ns = this.ns.resolveNamespace(nsPath)
, prereqs = this.prereqs.slice(); // Get a copy to work with

pattern = this.pattern;
source = this.source;

if (typeof source == 'string') {
src = Matcher.getSource(name, pattern, source);
// TODO: Write a utility function that appends a
// taskname to a namespace path
var completeNamespace = function(s){
return nsPath.split(':').filter(function (item) {
return !!item;
}).concat(s).join(':');
};

// Deal with the various things the source argument can be:
// 1: A function
if (typeof source == 'function') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not completely sold on the function idea, mostly because part of the idea here was to make Rules behave more like Tasks. Could you leave this change for a different PR, and maybe implement it for both Tasks and Rules?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mde I think that this is just a more explicit version of the else { that was already in the codebase (https://github.com/jakejs/jake/pull/340/files/afe002d343308d61527ec894e78630889f406281#diff-03fda114c6cfeff1993dbc9c03d7724bL255) (exact body) - just more explicit about the conditions.

source = source(name);

if (source instanceof Array) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like duplcate logic of what's below. Can you refactor this so that it does its function stuff first, then do the regular logic for Array/String?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a similar thought - but then noticed each instance works slightly differently.

The if string outside of if function uses the matcher:

src = completeNamespace( Matcher.getSource(name, pattern, source) )

while if string inside of if function just uses completeNamespace:

src = completeNamespace(source);

(same for if array)

From skimming the source, I think that means if a string is passed directly, it can contain wildcards, but if it's returned from a function, it must be the entire filename of the source, without wildcards, etc.

Now, maybe wildcards should always be applied (or I've misinterpreted the code I skimmed), but that's my 2 cents.

// It's a function that returns an array of sources (strings)
src = [];
source.forEach(function(srcName){
src.push(completeNamespace( srcName) );
});
prereqs = prereqs.concat(src);
}

else if (typeof source == 'string') {
// It's a function that returns a single source (string)
src = completeNamespace(source);
prereqs.unshift(src);
}

else {
// It's a function that returns a headache.
debugger;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove debugger statement.

throw new Error("Can't parse rule source.");
}
}
else {
src = source(name);

// 2: An array of sources
else if (source instanceof Array) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instanceof is not a reliable check for Array. Prefer Array.isArray.

// Find them all:
src = [];
source.forEach(function(srcName){
src.push(completeNamespace( Matcher.getSource(name, pattern, srcName) ));
});
prereqs = prereqs.concat(src);
}

// 3: A single source
else if (typeof source == 'string') {
src = completeNamespace( Matcher.getSource(name, pattern, source) );
prereqs.unshift(src);
}

// TODO: Write a utility function that appends a
// taskname to a namespace path
src = nsPath.split(':').filter(function (item) {
return !!item;
}).concat(src).join(':');
// 4: There is no #4
else {
throw new Error("Can't parse rule source.");
}

// Generate the prerequisite for the matching task.
// It is the original prerequisites plus the prerequisite
// It is the original prerequisites plus the definied prerequisite/prerequisites
Copy link

@CodeLenny CodeLenny May 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definied

If changes are being made, might as well fix a typo in the new code?

// representing source file, i.e.,
//
// rule( '%.o', '%.c', ['some.h'] ...
//
// If the objective is main.o, then new task should be
//
// file( 'main.o', ['main.c', 'some.h' ] ...
prereqs = this.prereqs.slice(); // Get a copy to work with
prereqs.unshift(src);

// Prereq should be:
// 1. an existing task
Expand Down Expand Up @@ -307,7 +348,14 @@ Rule.prototype = new (function () {
tNs = jake.currentNamespace;
jake.currentNamespace = ns;
createdTask = jake.createTask('file', name, prereqs, action, opts);
createdTask.source = src.split(':').pop();
if (src instanceof Array) {
createdTask.source = [];
src.forEach(function(s){
createdTask.source.push(s.split(':').pop());
});
} else {
createdTask.source = src.split(':').pop();
}
jake.currentNamespace = tNs;

return createdTask;
Expand Down
83 changes: 83 additions & 0 deletions test/Jakefile.rule
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ task('default', ['tmp']);
directory('tmpsrc');
directory('tmpbin');


////////////////////////////////////////////////////////////
// Simple Suffix Rule
file('tmp', ['tmp_init', 'tmp_dep1.o', 'tmp_dep2.o'], function (params) {
Expand Down Expand Up @@ -112,6 +113,54 @@ task('tmp_src_init', ['tmpsrc'], function () {
}, {async: true});
////////////////////////////////////////////////////////////

//////////////////////////////////////////////////////
// Rule having multiple sources

////////////////////////////////////////////////////////////
// Multiple Source Rule tests
task('tmp_ms', [
'tmpbin',
'tmpbin/bar123.glom',
'sourceFunction:tmpbin/foo123.glom'
], function () {
console.log('multiple source rules task');
var data1 = fs.readFileSync('tmpbin/foo123.glom');
var data2 = fs.readFileSync('tmpbin/bar123.glom');
fs.writeFileSync('tmp_ms', data1 + data2 + ' multiple source rules');
});

+function(){
var srcFuncMulti = function (taskName) {
var list = [];

list.push(taskName.replace('tmpbin', 'tmpsrc').replace(/\.[^.]+$/, '.1') );
list.push(taskName.replace('tmpbin', 'tmpsrc').replace(/\.[^.]+$/, '.2') );
list.push(taskName.replace('tmpbin', 'tmpsrc').replace(/\.[^.]+$/, '.3') );

return list;
};

rule(/.*\.glom$/, srcFuncMulti, function () {
var data = fs.readFileSync(this.source[0]);
data += fs.readFileSync(this.source[1]);
data += fs.readFileSync(this.source[2]);
fs.writeFileSync(this.name, data.toString());
console.log('created .glom');
});
}();

[1, 2, 3].forEach(function (key) {
var name = 'tmpsrc/bar123.' + key;
file(name, ['tmpsrc'], function () {
fs.writeFile(name, name+ "\n", function (err) {
if (err) {
throw err;
}
console.log(name + ' task');
complete();
});
}, {async: true});
});

////////////////////////////////////////////////////////////
// Namespace Test. This is a Mixed Test.
Expand Down Expand Up @@ -227,6 +276,7 @@ namespace('regexPattern', function () {
namespace('sourceFunction', function () {

var srcFunc = function (taskName) {
/* console.log(taskName.replace(/\.[^.]+$/, '.txt')); */
return taskName.replace(/\.[^.]+$/, '.txt');
};

Expand All @@ -239,8 +289,41 @@ namespace('sourceFunction', function () {
var data = fs.readFileSync(this.source);
fs.writeFileSync(this.name, data.toString());
});

var srcFuncMulti = function (taskName) {
var list = [];

list.push(taskName.replace('tmpbin', 'tmpsrc').replace(/\.[^.]+$/, '.1') );
list.push(taskName.replace('tmpbin', 'tmpsrc').replace(/\.[^.]+$/, '.2') );
list.push(taskName.replace('tmpbin', 'tmpsrc').replace(/\.[^.]+$/, '.3') );

return list;
};

// Testing multiple rule sources returned by a function:

rule(/.*\.glom$/, srcFuncMulti, function () {
var data = fs.readFileSync(this.source[0]);
data += fs.readFileSync(this.source[1]);
data += fs.readFileSync(this.source[2]);
fs.writeFileSync(this.name, data.toString());
console.log('created .glom');
});

[1, 2, 3].forEach(function (key) {
var name = 'tmpsrc/foo123.' + key;
file(name, ['tmpsrc'], function () {
fs.writeFile(name, name+ "\n", function (err) {
if (err) {
throw err;
}
console.log(name + ' task');
complete();
});
}, {async: true});
});

});
////////////////////////////////////////////////////////////
task('clean', function () {
utils.file.rmRf('./foo');
Expand Down
29 changes: 29 additions & 0 deletions test/rule.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var cleanUpAndNext = function (callback) {
var tmpFiles = [
'tmp'
, 'tmp_ns'
, 'tmp_ms'
, 'tmp_cr'
, 'tmp_p'
, 'tmp_pf'
Expand Down Expand Up @@ -141,6 +142,34 @@ var tests = {
});
}

, 'test rule w multiple source': function (next) {
h.exec( '../bin/cli.js -f Jakefile.rule tmp_ms', function (out) {
debugger;
var output = [
"tmpsrc/bar123.1 task"
, "tmpsrc/bar123.2 task"
, "tmpsrc/bar123.3 task"
, "created .glom"
, "tmpsrc/foo123.1 task"
, "tmpsrc/foo123.2 task"
, "tmpsrc/foo123.3 task"
, "created .glom"
, "multiple source rules task" ];
var data;
assert.equal( output.join('\n') , out);
data = fs.readFileSync(process.cwd() + '/tmp_ms');
assert.equal([
'tmpsrc/foo123.1',
'tmpsrc/foo123.2',
'tmpsrc/foo123.3',
'tmpsrc/bar123.1',
'tmpsrc/bar123.2',
'tmpsrc/bar123.3',
' multiple source rules'].join("\n"), data.toString());
cleanUpAndNext(next);
});
}

};

['precedence', 'regexPattern', 'sourceFunction'].forEach(function (key) {
Expand Down