[PATCH] Add tracking to check for changes in exclude files - WAS - Re: Can we still manually create a .gitignore?

1 view
Skip to first unread message

Imran M Yousuf

unread,
Aug 11, 2009, 4:54:05 AM8/11/09
to nb...@googlegroups.com
This is done so that PathPattern cache for exclusion patterns can be evicted on
change/modified event of the file. It tracks both FileSystem API
changes and local
file system change.

Next step would be to ensure Turbo cache is updated.

Signed-off-by: Imran M Yousuf <imyo...@smartitengineering.com>
---
src/org/nbgit/util/exclude/Excludes.java | 76 +++++++++++++++++++++++++++--
1 files changed, 70 insertions(+), 6 deletions(-)

diff --git a/src/org/nbgit/util/exclude/Excludes.java
b/src/org/nbgit/util/exclude/Excludes.java
index 9b7cba3..796e232 100644
--- a/src/org/nbgit/util/exclude/Excludes.java
+++ b/src/org/nbgit/util/exclude/Excludes.java
@@ -45,14 +45,23 @@
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
+import java.util.Collections;
+import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
+import java.util.Timer;
+import java.util.TimerTask;
import java.util.Vector;
import org.nbgit.Git;
import org.netbeans.api.project.ProjectManager;
import org.netbeans.api.queries.SharabilityQuery;
+import org.openide.filesystems.FileAttributeEvent;
+import org.openide.filesystems.FileChangeAdapter;
+import org.openide.filesystems.FileEvent;
+import org.openide.filesystems.FileObject;
+import org.openide.filesystems.FileRenameEvent;
import org.openide.filesystems.FileUtil;
import org.spearce.jgit.lib.Repository;

@@ -67,19 +76,28 @@ private Excludes() {
}

private static final String FILENAME_GITIGNORE = ".gitignore"; // NOI18N
- private static HashMap<String, List<PathPattern>> ignorePatterns;
+ private final static HashMap<String, List<PathPattern>>
ignorePatterns = new HashMap<String, List<PathPattern>>();
+ private final static Set<File> monitoredFiles = new HashSet<File>();
+ private final static Timer timer = new Timer();
+ private final static int DELAY = 5000;

private static List<PathPattern> getIgnorePatterns(File file) {
- if (ignorePatterns == null) {
- ignorePatterns = new HashMap<String, List<PathPattern>>();
- }
- String key = file.getAbsolutePath();
+ final String key = file.getAbsolutePath();
List<PathPattern> patterns = ignorePatterns.get(key);
- if (patterns == null) {
+ if (patterns == null && file.exists() && !file.isDirectory()) {
patterns = readIgnorePatterns(file);
if (!patterns.isEmpty()) {
ignorePatterns.put(key, patterns);
}
+ if(!monitoredFiles.contains(file)) {
+ synchronized(monitoredFiles) {
+ monitoredFiles.add(file);
+ }
+ monitorFileChange(file, key);
+ }
+ }
+ else if (patterns == null) {
+ patterns = Collections.emptyList();
}
return patterns;
}
@@ -162,6 +180,52 @@ public static boolean isIgnored(File file,
boolean checkSharability) {
return false;
}

+ private static void monitorFileChange(final File file, final String key) {
+ final FileObject fileObject = FileUtil.toFileObject(file);
+ TimerTask task = new TimerTask() {
+
+ private Date lastModified = fileObject.lastModified();
+
+ @Override
+ public void run() {
+ if (!lastModified.equals(fileObject.lastModified())) {
+ fileObject.refresh();
+ lastModified = fileObject.lastModified();
+ }
+ }
+ };
+ timer.schedule(task, DELAY, DELAY);
+ fileObject.addFileChangeListener(new FileChangeAdapter() {
+
+ private void evictPatterns() {
+ synchronized (ignorePatterns) {
+ ignorePatterns.remove(key);
+ }
+ }
+
+ @Override
+ public void fileChanged(FileEvent fe) {
+ evictPatterns();
+ }
+
+ @Override
+ public void fileAttributeChanged(FileAttributeEvent fe) {
+ System.out.println("A");
+ evictPatterns();
+ }
+
+ @Override
+ public void fileDeleted(FileEvent fe) {
+ evictPatterns();
+ }
+
+ @Override
+ public void fileRenamed(FileRenameEvent fe) {
+ evictPatterns();
+ }
+ });
+ }
+
private static String stripWorkDir(File wd, File f) {
int skip = f.getPath().length() > wd.getPath().length() ? 1 : 0;
String relName = f.getPath().substring(wd.getPath().length() + skip);
--
1.6.2.1

On Tue, Aug 11, 2009 at 12:41 PM, Imran M Yousuf<imyo...@gmail.com> wrote:
> On Tue, Aug 11, 2009 at 10:10 AM, Jonas Fonseca<jonas....@gmail.com> wrote:
>>
>> On Mon, Aug 10, 2009 at 21:43, Imran M Yousuf <imyo...@gmail.com> wrote:
>>>
>>> On Mon, Aug 10, 2009 at 10:46 PM, bhauff<bha...@gmail.com> wrote:
>>> >
>>> > I posted a comment in the .gitignore issue.  Basically, any changes I
>>> > make to the .gitignore will not be reflected until I close and reopen
>>> > Netbeans.
>>>
>>> I think for this case the following could be helpful -
>>> http://bits.netbeans.org/dev/javadoc/org-openide-filesystems/org/openide/filesystems/FileUtil.html#addFileChangeListener(org.openide.filesystems.FileChangeListener,%20java.io.File)
>>
>> It says:
>>
>>  "When attached to a file it listens for file changes (due to saving
>> from inside NetBeans) and for deletes and renames. "
>>
>> Which means that it won't pick up changes made outside of the IDE. So
>> I think we still need to have something tracking the
>> File.lastModified() timestamp or similar. I am currently working on a
>> per repository PathPattern cache, which should hopefully fix this
>> issue, provide a good speed up, and cleanup some of the code.
>>
>
> Thats true but even if we support it being updated from within
> NetBeans thats a cool thing; furthermore adding a timer task to
> refresh the FileObject will also do the trick. I am already working on
> it and will hopefully have a patch ready soon! The pattern cache idea
> seems cool!
>
> - Imran
>
>> --
>> Jonas Fonseca
>>
>> >>
>>
>
>
>
> --
> Imran M Yousuf
> Entrepreneur & Software Engineer
> Smart IT Engineering
> Dhaka, Bangladesh
> Email: im...@smartitengineering.com
> Blog: http://imyousuf-tech.blogs.smartitengineering.com/
> Mobile: +880-1711402557
>



--
Imran M Yousuf
Entrepreneur & Software Engineer
Smart IT Engineering
Dhaka, Bangladesh
Email: im...@smartitengineering.com
Blog: http://imyousuf-tech.blogs.smartitengineering.com/
Mobile: +880-1711402557

Jonas Fonseca

unread,
Aug 11, 2009, 9:13:24 AM8/11/09
to nb...@googlegroups.com
Thanks,

Some comments ...

On Tue, Aug 11, 2009 at 04:54, Imran M Yousuf<imyo...@gmail.com> wrote:
> @@ -67,19 +76,28 @@ private Excludes() {
>     }
>
>     private static final String FILENAME_GITIGNORE = ".gitignore"; // NOI18N
> -    private static HashMap<String, List<PathPattern>> ignorePatterns;
> +    private final static HashMap<String, List<PathPattern>> ignorePatterns = new HashMap<String, List<PathPattern>>();
> +    private final static Set<File> monitoredFiles = new HashSet<File>();
> +    private final static Timer timer = new Timer();
> +    private final static int DELAY = 5000;
>
>     private static List<PathPattern> getIgnorePatterns(File file) {
> -        if (ignorePatterns == null) {
> -            ignorePatterns = new HashMap<String, List<PathPattern>>();
> -        }
> -        String key = file.getAbsolutePath();
> +        final String key = file.getAbsolutePath();
>         List<PathPattern> patterns = ignorePatterns.get(key);
> -        if (patterns == null) {
> +        if (patterns == null && file.exists() && !file.isDirectory()) {
>             patterns = readIgnorePatterns(file);
>             if (!patterns.isEmpty()) {
>                 ignorePatterns.put(key, patterns);
>             }

Shouldn't access to ignorePatterns here also be synchronized? Maybe we
should just use a thread-safe Map instead.

> +            if(!monitoredFiles.contains(file)) {
> +                synchronized(monitoredFiles) {
> +                    monitoredFiles.add(file);
> +                }

I don't understand the purpose of monitoredFiles. You never remove
entries from it. To me it would be simpler if the ignorePatterns map
serve the dual purpose of tracking both cached and monitored files?

> +                monitorFileChange(file, key);
> +            }
> +        }
> +        else if (patterns == null) {
> +            patterns = Collections.emptyList();
>         }
>         return patterns;
>     }
> @@ -162,6 +180,52 @@ public static boolean isIgnored(File file,
> boolean checkSharability) {
>         return false;
>     }
>
> +    private static void monitorFileChange(final File file, final String key) {
> +        final FileObject fileObject = FileUtil.toFileObject(file);
> +        TimerTask task = new TimerTask() {
> +
> +            private Date lastModified = fileObject.lastModified();
> +
> +            @Override
> +            public void run() {
> +                if (!lastModified.equals(fileObject.lastModified())) {
> +                    fileObject.refresh();
> +                    lastModified = fileObject.lastModified();
> +                }
> +            }
> +        };
> +        timer.schedule(task, DELAY, DELAY);

Do you think it wold be worth it to have just one timer task to
iterate over the cached ignore files?

> +        fileObject.addFileChangeListener(new FileChangeAdapter() {
> +
> +            private void evictPatterns() {
> +                synchronized (ignorePatterns) {
> +                    ignorePatterns.remove(key);
> +                }
> +            }
> +
> +            @Override
> +            public void fileChanged(FileEvent fe) {
> +                evictPatterns();
> +            }
> +
> +            @Override
> +            public void fileAttributeChanged(FileAttributeEvent fe) {
> +                System.out.println("A");

Some debug left overs ...

> +                evictPatterns();
> +            }
> +
> +            @Override
> +            public void fileDeleted(FileEvent fe) {
> +                evictPatterns();
> +            }
> +
> +            @Override
> +            public void fileRenamed(FileRenameEvent fe) {
> +                evictPatterns();
> +            }
> +        });
> +    }
> +
>     private static String stripWorkDir(File wd, File f) {
>         int skip = f.getPath().length() > wd.getPath().length() ? 1 : 0;
>         String relName = f.getPath().substring(wd.getPath().length() + skip);
> --
> 1.6.2.1

BTW, last year I filed a bug for JGit about supporting file
monitoring. It has some interesting comments by Robin about the
reloading done internally in JGit.

- http://code.google.com/p/egit/issues/detail?id=40

--
Jonas Fonseca

Imran M Yousuf

unread,
Aug 11, 2009, 9:38:32 PM8/11/09
to nb...@googlegroups.com
Thank you for the comments, I have also added some :).
Yes it should be synchronized as well, but since it was not done
originally I did not change it as *it works*. But I think I will re-do
the patch by simply changing it to Hashtable.

>> +            if(!monitoredFiles.contains(file)) {
>> +                synchronized(monitoredFiles) {
>> +                    monitoredFiles.add(file);
>> +                }
>
> I don't understand the purpose of monitoredFiles. You never remove
> entries from it. To me it would be simpler if the ignorePatterns map
> serve the dual purpose of tracking both cached and monitored files?
>

hmm...stupid of me should have noticed it myself. I agree with the comments.
I gave it a thought too and even started implementing it :) but the
reason I did not submit it was trade-off with number of simple threads
with complexity and iteration of checking a set of files. I am still
not sure which one would be better - several/many small simple threads
or one thread with high degree of iteration running continuously in
background. What is your take on it?

>> +        fileObject.addFileChangeListener(new FileChangeAdapter() {
>> +
>> +            private void evictPatterns() {
>> +                synchronized (ignorePatterns) {
>> +                    ignorePatterns.remove(key);
>> +                }
>> +            }
>> +
>> +            @Override
>> +            public void fileChanged(FileEvent fe) {
>> +                evictPatterns();
>> +            }
>> +
>> +            @Override
>> +            public void fileAttributeChanged(FileAttributeEvent fe) {
>> +                System.out.println("A");
>
> Some debug left overs ...
>

Sorry for that :(, will remove them.

>> +                evictPatterns();
>> +            }
>> +
>> +            @Override
>> +            public void fileDeleted(FileEvent fe) {
>> +                evictPatterns();
>> +            }
>> +
>> +            @Override
>> +            public void fileRenamed(FileRenameEvent fe) {
>> +                evictPatterns();
>> +            }
>> +        });
>> +    }
>> +
>>     private static String stripWorkDir(File wd, File f) {
>>         int skip = f.getPath().length() > wd.getPath().length() ? 1 : 0;
>>         String relName = f.getPath().substring(wd.getPath().length() + skip);
>> --
>> 1.6.2.1
>
> BTW, last year I filed a bug for JGit about supporting file
> monitoring. It has some interesting comments by Robin about the
> reloading done internally in JGit.
>
>  - http://code.google.com/p/egit/issues/detail?id=40
>

Nice! Seems like we are on the right track...

Imran M Yousuf

unread,
Aug 11, 2009, 10:33:48 PM8/11/09
to nb...@googlegroups.com
On Tue, Aug 11, 2009 at 3:54 PM, Imran M Yousuf<imyo...@gmail.com> wrote:
> This is done so that PathPattern cache for exclusion patterns can be evicted on
> change/modified event of the file. It tracks both FileSystem API
> changes and local
> file system change.
>
> Next step would be to ensure Turbo cache is updated.
>

Can you please help me out on how I could clear the cache? BTW I
resubmitted the patch.

I was thinking of something like this - whenever a pattern is added
something should be added to the cache and whenever a pattern is
removed it should be done so from the cache as well. Whenever a new
file is added to the index from NetBeans or file system it should be
checked whether it should be excluded or not and added to the cache if
required and when a file is deleted so should it be done from cache as
well; all in all a simple read-write through cache strategy.

- Imran

Jonas Fonseca

unread,
Aug 12, 2009, 8:51:02 AM8/12/09
to nb...@googlegroups.com

I am not completely sure I follow you. If "cache" refers to the
exclude pattern cache, then yes, we want to pick up changes of
.gitignore and .git/info/exclude files and mirror it.

From what I understand, you want to also refresh or update the status
information cache used for showing the status view and annotating the
files in the file explorers. So when an .gitignore file has changed,
say in directory src/org/example/, all files under that directory
(which the .gitignore file has an effect on), needs to be refreshed as
well.

The way to do this is to call StatusCache's refreshCached() method of
GitUtils's forceStatusRefresh() ... on a background thread, which I
guess the pattern cache update code is already running on.

Regarding file deletion, deletion of .gitignore files is similar to
changes of .gitignore files, the module's VCSIntercepter will pick up
deletions of tracked files done via the IDE and will refresh the
status cache accordingly.

Don't know if that answers your question.


--
Jonas Fonseca

Reply all
Reply to author
Forward
0 new messages