Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#15484 closed defect (fixed)

Dojo build system fails when the file path contains a dot

Reported by: dson Owned by: Rawld Gill
Priority: undecided Milestone: 1.8
Component: BuildSystem Version: 1.7.2
Keywords: Cc:
Blocked By: Blocking:

Description

The Dojo build system fails when there is a dot somewhere in the absolute file path. For example, if the build is located in /path/to/build the build will finish without errors. However if the build is located in /path/.to/build the build will fail. Note that the profile does not contain absolute paths, but I guess the build system resolves relative paths to absolute paths before processing.

The problem can easily be reproduced with the base profile. If the absolute path contains a dot, the result is an empty dojo/dojo layer, and the build script will report 2 errors, directly after "starting writing resources":

error(303) Missing include module for layer. missing: dojo/main; layer: 0

error(303) Missing include module for layer. missing: dojo/main; layer: 0

Find attached the build script output and the generated build report for both cases.

(Someone else already reported this issue in 1.7.0: http://dojo-toolkit.33424.n3.nabble.com/1-7-0-Custom-build-within-a-dot-directory-not-working-td3468672.html, yet I couldn't find a related ticket.)

Attachments (5)

normal-output.txt (2.7 KB) - added by dson 7 years ago.
Output when directory does not contain a dot
output-with-dot.txt (1.8 KB) - added by dson 7 years ago.
Output when directory contains a dot
dot-in-patch.patch (2.8 KB) - added by Ben Lowery 7 years ago.
dot-in-patch2.patch (2.5 KB) - added by dson 7 years ago.
This patch also works for builds using Java
dotfix.patch (5.2 KB) - added by Rawld Gill 7 years ago.
see comment 15

Download all attachments as: .zip

Change History (25)

Changed 7 years ago by dson

Attachment: normal-output.txt added

Output when directory does not contain a dot

Changed 7 years ago by dson

Attachment: output-with-dot.txt added

Output when directory contains a dot

comment:1 Changed 7 years ago by dson

A simple fix would be to rename the dot directory. However the build is run on a Jenkins build server, I do not have access to. The Jenkins home directory is by default set to ~/.jenkins.

Changed 7 years ago by Ben Lowery

Attachment: dot-in-patch.patch added

comment:2 Changed 7 years ago by Ben Lowery

We hit the same thing. Attached a patch that should fix it. I'm under CLA.

comment:3 Changed 7 years ago by dson

Thanks for the patch! Unfortunately the patch introduces another bug when using Rhino instead of Node. The joinPath method in the patch returns a path ending in a '/' when the second part is an empty string. This causes the readdirSync method in build/rhino/fs.js (line 51) to strip the first character of file names, resulting in errors such as:

cp: cannot stat '/mnt/data/dev/build/workspace/bin/client/source/dojo/ackage.json': No such file or directory

I replaced the joinPath method with

joinPath : function(a, b) {
    return a.replace(/\/?$/, b ? "/" + b : "");
}

which will also make it work for Java builds.

Last edited 7 years ago by dson (previous) (diff)

Changed 7 years ago by dson

Attachment: dot-in-patch2.patch added

This patch also works for builds using Java

comment:4 Changed 7 years ago by Ben Lowery

Ah, we did this:

diff --git a/util/build/rhino/fs.js b/util/build/rhino/fs.js
index 5b8974c..5138c44 100644
--- a/util/build/rhino/fs.js
+++ b/util/build/rhino/fs.js
@@ -45,10 +45,9 @@ define([], function() {
                readFileSync: readFileSync,
 
                readdirSync: function(path) {
-                       // java returns the complete path with each filename in listFiles; node returns just the filename
                        // the item+"" is necessary because item is a java object that doesn't have the substring method
-                       var l= path.length + 1;
-                       return (new java.io.File(path)).listFiles().map(function(item){ return (item+"").substring(l); });
+                       // We use the name getter here to get just the name of the current item, instead of the full path
+                       return (new java.io.File(path)).listFiles().map(function(item){ return (item.name+""); });
                },
 
                readFile: function(filename, encoding, cb) {

comment:5 Changed 7 years ago by dson

Even better, thanks!

comment:6 Changed 7 years ago by Rawld Gill

Milestone: tbd1.8
Status: newassigned

comment:7 Changed 7 years ago by Rawld Gill

Resolution: worksforme
Status: assignedclosed

As I understand the patches, they are attempting to circumvent the excludes logic. Why not just use it as intended? Please see http://dojotoolkit.org/reference-guide/1.7/build/buildSystem.html#specifying-resources for more information.

By default, the excludes logic will exclude any filenames testing positive with /(\/\.)|(~$)/. Therefore, paths with a dot are excluded.

You can explicitly set the exclude regex by providing a trees config in the package config. For example, let's say I want to build a package named "myPackage", and that package contains dots in the path, then I could do something like this:

packages: [
	{
		name:"myPackage",
		location:"path/.to/my/package",
		trees:[
			[".", ".", /$^/]
		]
	},
	// other packages follow...

The regex given above never tests positive for anything, so nothing will be excluded. You could/should tune this regex to your particular need.

There is an undocumented trick to get the same effect which is to provide an explicit falsy value for the regex. Note that undefined is not considered an explicit falsy value. For example:

trees:[
	[".", ".", 0]
]

Lastly, the excludes item can also be a function that maps a file name into a boolean, with true indicating the file should be excluded.

fwiw, I tested examples with both source and destinations with dots on both node and rhino and everything seemed to work.

comment:8 Changed 7 years ago by Ben Lowery

Resolution: worksforme
Status: closedreopened

I think you missed the broken case here. The problem is that if you checkout Dojo and friends to a working that contains a . in the parent path, the build breaks. For instance, check out to /tmp/.broken/dojo and try a build there. Modifying the provided package spec is not a viable way forward.

comment:9 Changed 7 years ago by Ben Lowery

Also, the provided patches allow the defect to *only* match against the part of the path relative to the base of the project. So you can easily exclude things that start with a . while still building inside a dir that starts with a .

comment:10 Changed 7 years ago by dante

+1 blowery: asinine to require a user to setup special mechanics to simply put a Dojo checkout in a folder that happens to have a . somewhere in the parent. In the case of both reports, I believe Jenkins likes to put it's stuff in ~/.jenkins/ ...

comment:11 Changed 7 years ago by Rawld Gill

Status: reopenedassigned

The solution is the same: use excludes as it was designed to be used. You do not need to modify the profile. Instead, construct a small profile that modifies the stock dojo profile. For example:

var profile = {
	packages:[{
		name:"dojo",
		trees:[[".", ".", function(filename){return /(\/\.)|(~$)/.test(filename.replace("/.dtk/", "/"));}]]
	}]
};

Say the above was stored at ~/withdots.profile.js, then you could build base like this:

./build.sh -p base -p ~/withdots -r

The withdots profile has the effect of modifying just the trees switch in the stock dojo profile. I tested this and it works fine.

You could also put the modification to the dojo package in the the build profile you're using for some particular application.

It is fairly unusual to checkout to a directory that starts with a dot as this is typically a hidden directory.

I don't have a big problem with the idea behind the patch (other than it's fixing something that already works and therefore might break something else). I'll keep this open so I can take another look.

comment:12 in reply to:  10 Changed 7 years ago by Rawld Gill

Replying to dante:

+1 blowery: asinine to require a user to setup special mechanics to simply put a Dojo checkout in a folder that happens to have a . somewhere in the parent. In the case of both reports, I believe Jenkins likes to put it's stuff in ~/.jenkins/ ...

hmmm....first this issue is not a ". somewhere in the parent", but rather a dot as the first character of a file name segment. Typically, this means a hidden files by default. It does not seem irrational to ignore hidden files. The proof is in the pudding, after nearly a year of use we know of two cases where folks have tried this...clearly indicating its not something very common.

I can see the argument that the user is specifying a directory that begins with a dot, and therefore that intent ought to override the default behavior. But, like I said above, there is a simple solution if you want to do something unusual.

comment:13 Changed 7 years ago by dante

The issue IS with a . in a parent. You can call it a "file name segment" if you want, but what is really happening is Dojo build is failing when any one of those "segments" (I call it a parent folder/directory) contains a dot as the first character. I absolutely agree this indicates a "hidden file". Lots of programs use "hidden files" to store their sandboxed user-config files so they don't appear in "normal file listings". I'm not trying to name my namespace ".foo", I'm trying to put an entire program/build system inside a folder, which happens to be a hidden folder where some installed helper thinger stores it's tmp stuff while it does it's thing.

Additionally, I'm not "specifying a directory that begins with a dot". If I cwd into util/build and run like that, what difference should it make where in the filesystem I happen to be? It does not seem irrational to ignore hidden files IN THE TREE I AM BUILDING, but when my path is:

/home/dante/foo/bar/baz/.hahahahyoucantfindme/project/website/js/util/buildscripts

I'd expect to not have to do any undocumented-special-profile-making to accomodate it.

Regarding you comment about pudding: We (work) *JUST* made the jump to 1.7 a few weeks ago. A bunch of the initial regressions prevented us from migrating immediately to 1.7.0. This is one of the things in 1.7.2 that "broke", out of the box, for something we have been doing for a while. That is a regression.

The issue is: Dojo build should not care about anything except itself: paths are all relative. If my "path segments" have a dot in them, why should Dojo care? If my namespace contains folders that begin with dots is an entirely different issue. I assume Dojo build ignores them (.svn/.DS_STORE anyone?), which is fine because that's the way it always has been. If I want to

$ mv dojotrunk/ .dojotrunk && cd .dojotrunk/util/buildscripts && build.sh profile=~/dotsshouldntmatter 

I should be able to. And still be able to run the same build. Using relative paths.

comment:14 Changed 7 years ago by Ben Lowery

Miss you Pete.

comment:15 Changed 7 years ago by Rawld Gill

Since the sleep gods decided to frown on me tonight, I worked up a fix which I'm attaching next. Please give it a try. I need to test more later with fresh eyes before committing.

You'll see the main fix is in discover; the rest is a small tune-up to the *.profile.js resources. While working on this, I realized that the trees config is unnecessary in all our stock profiles. Further, with the discover fix, the excludes regex is wrong. So I removed trees from all the *.profile.js resources in dojo, dijit, dojox, util/doh, and demos.

Changed 7 years ago by Rawld Gill

Attachment: dotfix.patch added

see comment 15

comment:16 in reply to:  13 Changed 7 years ago by Rawld Gill

Replying to dante:

I fully understand and said as much in comment 20. I also said I'd look at the patch; that's why I kept the ticket open.

The issue IS with a . in a parent.

It's more-specific than that: it only occurs when a directory or filename starts with a dot. For example, there is no problem with path/to/this.or.that/dojo.

The issue is: Dojo build should not care about anything except itself: paths are all relative. If my "path segments" have a dot in them, why should Dojo care?

Right. I would describe it slightly differently...the issue is the root path of a tree should not cause an exclusion. For example, if a profile has a trees config like this:

  trees:["some/src/path", "some/dest/path", /* some excludes regex */]

Then it does not make sense for the regex to exclude all files that start with "some/src/path". That is what was happening.

The problem could have been fixed by simply changing the exclude regexs in the stock profiles. For example, I'm fairly sure changing the dojo.profile.js trees value to

trees:[".", ".", /.+\/dojo\/(.*)((\/\.)|(~$))/]

would also work. But that's a bit obtuse for my taste.

If my namespace contains folders that begin with dots is an entirely different issue. I assume Dojo build ignores them (.svn/.DS_STORE anyone?), which is fine because that's the way it always has been.

I believe the package author should be able to say what's ignored and what isn't. Perhaps a package designer wants to release namespaces with hidden names. The 1.7+ builder can do that. The dojo, dijit, dojox, util/doh, and demos profiles ignore all names that start with a dot...I think that's mostly the .svn names...and all names that end with ~...because those are editor backup names. This is slightly better than the 1.6- builder.

But, in any event, it's fully configurable in 1.7+; it wasn't before.

comment:17 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

In [29158]:

improved discovery to not pass base src path to exclude computation; removed unneeded trees config from all standard profiles; fixes #15484; !strict; thanks blowery

comment:18 Changed 7 years ago by Rawld Gill

In [29159]:

checked in wrong discover.js in [29158], this is the correct one; fixes #15484; !strict

comment:19 Changed 7 years ago by Rawld Gill

In [29160]:

one more profile fix; fixes #15484; !strict

comment:20 Changed 7 years ago by Rawld Gill

In [29246]:

one more profile improvment consequent to #15484; refs #15484

Note: See TracTickets for help on using tickets.