Opened 15 years ago
Closed 15 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 15 years ago by
Component: | General → Project : CKReleaser |
---|---|
Keywords: | Confirmed added |
Owner: | set to Wiktor Walc |
Changed 15 years ago by
Attachment: | 4939.patch added |
---|
Changed 15 years ago by
Attachment: | ckreleaser.jar added |
---|
comment:2 follow-up: 3 Changed 15 years ago by
Keywords: | Review? added |
---|
comment:3 Changed 15 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 15 years ago by
Keywords: | Review+ added; Review? removed |
---|
The isChildPath() function was invalid.
Apart from that, theoretically, there should be something like
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.