-
Notifications
You must be signed in to change notification settings - Fork 123
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.
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
copy/move directory updates #39
Conversation
…Empty static method to FilterChain. Added test to Move task to check issue that was reported regarding moving directories.
…if the Includes list is empty when the tasks are called.
…. Comments and additional tests needed.
… cleaned up the formatting and removed some commented codes.
… the FileOperationMap class. Updated the Log outputs in the copy and move tasks to take directory operations into account. Added more comments.
…ed documentation reference.
…nsideration. Also added comments.
This is trying to fix the problem reported in issue #11. |
…cted FileCopyMap property back in CopyTask (as obsolete property). Reworked Copy/Move tasks to move entire directoriesif CopyFileSet Includes list is empty. Added method to convert FileOperationMap to Hashtable (for FileCopyMap property). Added private static method to FileUtils static class to check if FilterChain is null or empty.
FYI - These changes were tested using mono-2.0, mono-4.0, net-2.0, net-3.5, and net-4.0 on WinXP and Mac OS X 10.6. |
/// destination file. | ||
/// </para> | ||
/// <para> | ||
/// On Windows, the <see cref="Hashtable" /> is case-insensitive. | ||
/// </para> | ||
/// </remarks> | ||
protected Hashtable FileCopyMap { | ||
get { return _fileCopyMap; } | ||
[ObsoleteAttribute("Use OperationMap instead of FileCopyMap.")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd make it clear that this is essentially a read-only hashtable now.
Any changes you make to the hashtable will not be taken into account.
I'd clarify this in the docs and the ObsoleteAttribute, and check if you can actually return a read-only hashtable (that will throw exception when you attend to modify it); does such a thing exist in .NET ?
…eSet class (as protected internal property). Added IsNullOrEmpty static method to FilterChain class as an internal method. Reworked MoveDirectory static method in FileUtils class for clarity. Removed copytask test that was no longer relevent. Added more move task tests. Documented copy/move task to include details about directories operations.
…ring build. Matches recent change to Makefile.
…n checking files, not directories. Fixed new MoveTask test to take recent DirectoryScanner.IsEverythingIncluded fix into account. Added additional MoveTask test and added DirectoryScanner test.
{ | ||
string stagePath = GetTempDirectoryName(); | ||
|
||
Directory.Move(sourceDirectory, stagePath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps add a try / catch or try / finally in which you ensure that - if the stagePath (still) exists, it is removed.
this avoids "leaking" stage paths.
…additional comments.
…the status of directory operations.
… but different casing (ie: C:\nant to C:\NAnt) on windows system. Added test for this change and additional comments for clarity.
Log(Level.Info, "Copying {0} file{1} to '{2}'.", fileCount, (fileCount != 1) ? "s" : "", ToFile); | ||
} else { | ||
Log(Level.Info, "Copying {0} file{1} to '{2}'.", fileCount, (fileCount != 1) ? "s" : "", ToDirectory); | ||
if (OperationMap.Count > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I'd have the following to start the method:
if (OperationMap.Count == 0)
return;
That way you don't have to indent the complete logic in that method.
foreach (DictionaryEntry fileEntry in FileCopyMap) { | ||
string destinationPath = (string) fileEntry.Key; | ||
string sourcePath = ((FileDateInfo) fileEntry.Value).Path; | ||
if (OperationMap.Count > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I'd have the following to start the method:
if (OperationMap.Count == 0)
return;
That way you don't have to indent the complete logic in that method.
…rectoryName methods to internal for the time being as suggested by Gert Driesen.
…ck moving files in subdirectories.
Rec'd ok from Gert via email this merge. Will merge sometime today. :) |
…opy files into bin/Debug or bin/Release nant_0.91~alpha2+dfsg-3_all.deb in Ubuntu 12.04 and earlier actually ignored these due to a bug However, nant 0.92~rc1+dfsg-2 in Ubuntu 12.10 fixes this bug (possibly nant/nant#39). Which makes nant time-consumingly copy these files when the aren't actually used. Tested removal of <copy> on both nant 0.91 and nant 0.92 Will be submitting this patch to prebuild project for comment though I suspect there's nobody there to pay attention.
…opy files into bin/Debug or bin/Release nant_0.91~alpha2+dfsg-3_all.deb in Ubuntu 12.04 and earlier actually ignored these due to a bug However, nant 0.92~rc1+dfsg-2 in Ubuntu 12.10 fixes this bug (possibly nant/nant#39). Which makes nant time-consumingly copy these files when the aren't actually used. Tested removal of <copy> on both nant 0.91 and nant 0.92 This change has been used without issue for OpenSimulator since Nov 14 2012 commit 90c6d2e
…opy files into bin/Debug or bin/Release nant_0.91~alpha2+dfsg-3_all.deb in Ubuntu 12.04 and earlier actually ignored these due to a bug However, nant 0.92~rc1+dfsg-2 in Ubuntu 12.10 fixes this bug (possibly nant/nant#39). Which makes nant time-consumingly copy these files when the aren't actually used. Tested removal of <copy> on both nant 0.91 and nant 0.92 Will be submitting this patch to prebuild project for comment though I suspect there's nobody there to pay attention.
…opy files into bin/Debug or bin/Release nant_0.91~alpha2+dfsg-3_all.deb in Ubuntu 12.04 and earlier actually ignored these due to a bug However, nant 0.92~rc1+dfsg-2 in Ubuntu 12.10 fixes this bug (possibly nant/nant#39). Which makes nant time-consumingly copy these files when the aren't actually used. Tested removal of <copy> on both nant 0.91 and nant 0.92 Will be submitting this patch to prebuild project for comment though I suspect there's nobody there to pay attention.
Fix for issue nant#38
My attempt on addressing the issue of copying/moving directories. Please review before merging into master.