Opened 16 years ago
Closed 16 years ago
#4939 closed Bug (fixed)
CKReleaser limits source and target directories too aggressively
| Reported by: | Clayton Coleman | Owned by: | Wiktor Walc |
|---|---|---|---|
| Priority: | Normal | Milestone: | |
| Component: | Project : CKReleaser | Version: | SVN (CKEditor) - OLD |
| Keywords: | Confirmed Review+ | Cc: |
Description
CKReleaser prevents the source and target directories from being different because io.isChildPath has an incorrect logical test. The default build (as specified in release.bat) works because target is a child path of source and because the _dev directory is ignored during copy.
Default command (from latest version of release.bat):
java -jar ckreleaser/ckreleaser.jar ckreleaser.release ../.. release "3.1 SVN" ckeditor_3.1_svn --run-before-release=langtool.bat
release.bat is in the <root>/_dev/releaser directory of the source tree, which means the source dir is <root> and the target directory is <root>/_dev/releaser/release
For 3rd parties using different build systems and different directory structures (for instance, where the build output must live in a directory outside of the source tree) it would be preferable to specify a command like:
java -jar ckreleaser/ckreleaser.jar customckreleaser.release path/to/ckeditor build/ckeditor <versionid> <filename>
Executing this command throws the exception:
Target directory must be located outside source directory
which is thrown by this line of releaser.js
468 if ( CKRELEASER.io.isChildPath( targetDir.getCanonicalPath(), sourceDir.getCanonicalPath() ) )
469 throw "Target directory must be located outside source directory";
This is incorrect - the target directory and source directory are not located underneath each other, which examination of isChildPath shows is because the return statement is too aggressive.
403 isChildPath : function( childPath, parentPath )
404 {
405 if ( childPath.length > parentPath.length )
406 return false;
407
408 var i = 0, max = Math.min( childPath.length(), parentPath.length() );
409
410 while ( i < max && childPath.charAt( i ) == parentPath.charAt( i ) )
411 i++;
412
413 return i != max;
414 }
If the working directory is c:\test, the source dir is c:\test\path\to\ckeditor, and the target dir is c:\test\build\ckeditor, then isChildPath should return false (the two are not child paths of each other). However, "return i != max" returns true for this scenario because the two paths match only up until character 7 (i == 7, max == 22). It should be "return i==max".
If isChildPath is adjusted to correctly check path parentage then ckreleaser can be used in different directory structures
Attachments (2)
Change History (7)
comment:1 Changed 16 years ago by
| Component: | General → Project : CKReleaser |
|---|---|
| Keywords: | Confirmed added |
| Owner: | set to Wiktor Walc |
Changed 16 years ago by
| Attachment: | 4939.patch added |
|---|
Changed 16 years ago by
| Attachment: | ckreleaser.jar added |
|---|
comment:2 follow-up: 3 Changed 16 years ago by
| Keywords: | Review? added |
|---|
comment:3 Changed 16 years ago by
The provided JAR worked in the scenarios I needed - running in a working directory outside of source where target was a child of the working directory.
Thanks
comment:4 Changed 16 years ago by
| Keywords: | Review+ added; Review? removed |
|---|

The isChildPath() function was invalid.
Apart from that, theoretically, there should be something like
if (CKRELEASER.io.isChildPath(target, source) && !CKRELEASER.release.isIgnoredPath(target)) throw "Target directory must be located outside the source directory";to check whether target dir is ignored. If yes, then we should allow using target directory in the source directory (otherwise we'll not be able to create a release inside of the _dev folder).
In reality, the "isIgnoredPath" function works in a special way and cannot be called like that. Instead, we're looping through all ignored directories and checking whether the target directory is a child folder of one of the ignored folders.