Ticket #4939 (closed Bug: fixed)
CKReleaser limits source and target directories too aggressively
| Reported by: | claycole | Owned by: | wwalc |
|---|---|---|---|
| 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
Change History
comment:1 Changed 3 years ago by wwalc
- Owner set to wwalc
- Keywords Confirmed added
- Component changed from General to Project : CKReleaser
comment:2 follow-up: ↓ 3 Changed 3 years ago by wwalc
- Keywords Review? added
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.
