threads / discuss / 13454

[egit / jgit] Implementation of a file tree iteration using ignore rules.

Subject: [egit / jgit] Implementation of a file tree iteration using ignore rules.

## tl;dr

4 messages between May 9, 2008 and May 11, 2008.

replies: 3people: 2as markdown or json

Florian Köberle· May 9, 2008, 13:20 UTC · lore
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi

I like the idea of a Java implementation of git and would like to contribute to the jgit/egit project.

In order to get familiar with the code I started to implement a command like tool which works like git using the jgit library. I implemented very simple versions of the commands "help", "init" and finally wanted to implement the "add" command. However, I didn't find any tools to determine the files which should be added.

So I implemented a factory which returns an Iterable<File> for the iteration over all the files in a directory.

For an example see the unit test testRealisticExample() in the class FileIterableFactoryForAddCommandTest: http://repo.or.cz/w/egit/florian.git?a=blob;f=org.spearce.jgit.test/tst/org/spearce/jgit/lib/fileiteration/FileIterableFactoryForAddCommandTest.java;h=d3c78f4422c708f26ccb56434053bb711fa3116b;hb=669fd814d34e2f989b5f8eedbcb0d5bcf9743ce7

You can view the patches online at: http://repo.or.cz/w/egit/florian.git?a=shortlog;h=refs/heads/mailinglist-patches-0

I signed all patches and formatted them with the code formatter as I should. It's ok for me to put the patches under a dual license between a 3-clause BSD and the EPL[*3*]. Currently all files have a GPL 2 notice. I hope that is ok.

If you want I will send the patches to the mailing list, but I don't know any automated way to create all the emails. I am not even sure if I will get them formatted correctly with Thunderbird 2. It would be cool if you could tell me how to send patches via command line.

I hope you like my patches,
Florian Koeberle
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIJE+I59ca4mzhfxMRAqlwAKCSp57SkqvVsBpdt8o3jL6zNdn0kACfeLnZ IHErO96fu2rdQcT+JpmroYU= =E+vF -----END PGP SIGNATURE-----

Shawn O. Pearce· May 10, 2008, 00:11 UTC · re: Florian Köberle · lore

Re: [egit / jgit] Implementation of a file tree iteration using ignore rules.

Florian Kberle <FloriansKarten@web.de> wrote:
> 
> I like the idea of a Java implementation of git and would like to
> contribute to the jgit/egit project.
Even better, we'd love to have you contribute!  :-)
 
> In order to get familiar with the code I started to implement a command
> like tool which works like git using the jgit library. I implemented
> very simple versions of the commands "help", "init"

This is an interesting start. Did you see the existing "Main" class in org.spearce.jgit/src/org/spearce/jgit/pgm? It sets up and invokes a TextBuiltin, which is sort of like the "Command" class you added in your first patch. Though TextBuiltins are created on-the-fly and thus are harder/impossible to use to format a "jgit help".

I think your approach of building up a table of commands is likely the better one long term, so I am interested in seeing the two unify, taking the best from each (from Main and MainProgram that is).

Please note that jgit is restricted to Java 5 APIs only right now. The "MainProgram" class you introduced uses Arrays.copyOfRange() which does not compile under Java 5. I guess it is new in Java 6?

> and finally wanted
> to implement the "add" command. However, I didn't find any tools to
> determine the files which should be added.

Right. We haven't implemented this properly yet. So I am very happy to see someone starting to approach this.

> So I implemented a factory which returns an Iterable<File> for the
> iteration over all the files in a directory.

Sadly this is a reimplementation of the already existing FileTreeIterator, which is meant to be used within a TreeWalk instance.

The TreeWalk API is meant to iterate over a working directory in canonical tree entry name ordering, so that we can walk not just a working directory but also the index file and one or more tree objects in parallel. We can even walk multiple working directories at once making directory differencing fairly simple.

What is missing here is really two things:
 #1)  Take .gitignore and .git/info/exclude (and other patterns) into
      account as WorkingTreeIterator (base class of FileTreeIterator)
      loops over the entries in a directory.
      Since .gitignore can be per-directory we may need to add rules
      as we enter into a subtree (createSubtreeIterator method)
      and pop rules as we exit a subtree.
      Fortunately the pop part is easy if the rules are held within
      the WorkingTreeIterator as instance members as pop is already
      dealt with up inside of TreeWalk by simply discarding the
      subtree instance and returning back to the parent instance.
  #2) Be able to edit one (or more) index files during a
      TreeWalk.  I am (sort of) in the middle of that work on my
      egit/spearce.git fork's dircache branch.

We already have a method for filtering entries during TreeWalk; its the TreeFilter API and its many subclasses. I wonder if the rules for say .gitignore could simply be implemented through this API and then allow a TreeFilter to be set directly on the WorkingTreeIterator to "pre-filter" the entries before they get returned for merging in the TreeWalk main loop.

I am certain we need to write new subclasses of TreeFilter to handle fnmatch(3C) style glob rules. But they shouldn't be too difficult. For example we already have PathFilter to perform equality testing on path names.

Building onto TreeFilter makes for some more interesting cases, as we can then feed globs into the revision machinary and actually do something like `jgit log -- 'path/*.c'`, with the globbing being done _on_the_fly_ at each tree, and not once up front by the shell.

 
Show 11 quoted lines
> For an example see the unit test testRealisticExample() in the class
> FileIterableFactoryForAddCommandTest:
> http://repo.or.cz/w/egit/florian.git?a=blob;f=org.spearce.jgit.test/tst/org/spearce/jgit/lib/fileiteration/FileIterableFactoryForAddCommandTest.java;h=d3c78f4422c708f26ccb56434053bb711fa3116b;hb=669fd814d34e2f989b5f8eedbcb0d5bcf9743ce7
> 
> You can view the patches online at:
> http://repo.or.cz/w/egit/florian.git?a=shortlog;h=refs/heads/mailinglist-patches-0
> 
> I signed all patches and formatted them with the code formatter as I
> should. It's ok for me to put the patches under a dual license between a
> 3-clause BSD and the EPL[*3*]. Currently all files have a GPL 2 notice.
> I hope that is ok.
Thanks.
I'll be writing a script to edit the headers to switch GPL notice to
EDL (3-clause BSD) notice real soon, and apply it to the bleeding
edge tree.  We may need to use it a few times to cover everyone's
topic branches before they merge into the main tree.
 
> If you want I will send the patches to the mailing list, but I don't
> know any automated way to create all the emails. I am not even sure if I
> will get them formatted correctly with Thunderbird 2. It would be cool
> if you could tell me how to send patches via command line.
I think the command you are looking for is `git send-email`.
 
-- 
Shawn.
Florian Köberle· May 10, 2008, 15:11 UTC · re: Shawn O. Pearce · lore

Re: [egit / jgit] Implementation of a file tree iteration using ignore rules.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi
| This is an interesting start.  Did you see the existing "Main" class
| in org.spearce.jgit/src/org/spearce/jgit/pgm?  It sets up and invokes
| a TextBuiltin, which is sort of like the "Command" class you added in
| your first patch.  Though TextBuiltins are created on-the-fly and thus
| are harder/impossible to use to format a "jgit help".
I noticed that the class appeared after a rebase, but didn't have a
closer look to it yet.
|
| I think your approach of building up a table of commands is likely
| the better one long term, so I am interested in seeing the two unify,
| taking the best from each (from Main and MainProgram that is).
|
| Please note that jgit is restricted to Java 5 APIs only right now.
| The "MainProgram" class you introduced uses Arrays.copyOfRange()
| which does not compile under Java 5.  I guess it is new in Java 6?
Yes it is new in Java 6. A patch fixing this is contained in the patch
set I send to the mailing list.
| What is missing here is really two things:
|
|  #1)  Take .gitignore and .git/info/exclude (and other patterns) into
|       account as WorkingTreeIterator (base class of FileTreeIterator)
|       loops over the entries in a directory.

I had a look at the WorkingTreeIterator and it seems to me that it is possible to reuse my Rules class there.

We could simply give the iterator a member variable of type Rule.

The method loadEntries of WorkingTreeIterator could then use the rules class to filter out unwanted files and directories.

The constructor WorkingTreeIterator(final WorkingTreeIterator p) could use the Rules#getRulesForSubDirectory to create a Rules instance from the parent Rules instance.

Also note that my Rules implementation would ignore the directory a in the case of "/a\n!/a/b.txt". This means that a directory may not appear in the list entries, but must be used to create another iterator.

I suggest to put all the classes from the package org.spearce.jgit.treewalk and the package org.spearce.jgit.lib.fileiteration into one package. Please tell me which package and I will send a patch, or do it yourself. I don't have any outstanding changes.

I don't see a easy way of porting my Rules implementation to the TreeFilter framework, but as you may noticed it is may not necessary to do so.

| I think the command you are looking for is `git send-email`.
Thanks, after switching to a newer version of git I was able to use that
command.
Best regards,
Florian
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIJbr659ca4mzhfxMRAn/ZAKCYERVJfHgZmvGFXEP+uCT0rD2RawCgqBIW Xa4NTcAWjt8j0oMPdKMjbbY= =koEN -----END PGP SIGNATURE-----

Shawn O. Pearce· May 11, 2008, 00:12 UTC · re: Florian Köberle · lore

Re: [egit / jgit] Implementation of a file tree iteration using ignore rules.

Florian Kberle <FloriansKarten@web.de> wrote:
Show 8 quoted lines
> | This is an interesting start.  Did you see the existing "Main" class
> | in org.spearce.jgit/src/org/spearce/jgit/pgm?  It sets up and invokes
> | a TextBuiltin, which is sort of like the "Command" class you added in
> | your first patch.  Though TextBuiltins are created on-the-fly and thus
> | are harder/impossible to use to format a "jgit help".
>
> I noticed that the class appeared after a rebase, but didn't have a
> closer look to it yet.

I guess you started working on an older version, only to later find out that I had also done a lot of work in the mean-time. :-)

My jgit contributions come in huge bursts.  RevWalk/TreeWalk was one
back in March; the transport API (almost 10,000 lines of code itself)
is the most recent from late April/early May.  Once its fully into
the mainline I'll probably have to slow down for a couple of months.
I have to move in July and have a lot of things to do between now
and then.
 
Show 6 quoted lines
> | Please note that jgit is restricted to Java 5 APIs only right now.
> | The "MainProgram" class you introduced uses Arrays.copyOfRange()
> | which does not compile under Java 5.  I guess it is new in Java 6?
>
> Yes it is new in Java 6. A patch fixing this is contained in the patch
> set I send to the mailing list.

To keep the history bisectable as much as possible it is better if you use `git rebase -i` to squash these two changes together, so that we never introduce the Java 6 usage into the codebase.

Show 7 quoted lines
> I had a look at the WorkingTreeIterator and it seems to me that it is
> possible to reuse my Rules class there.
> 
> We could simply give the iterator a member variable of type Rule.
> 
> The method loadEntries of WorkingTreeIterator could then use the rules
> class to filter out unwanted files and directories.
Yea, that sounds right.
 
> Also note that my Rules implementation would ignore the directory a in
> the case of "/a\n!/a/b.txt". This means that a directory may not appear
> in the list entries, but must be used to create another iterator.
Ouch.  I forgot about that fun corner case.  In the context of a
TreeWalk directory "a" must actually still be reported as an entry so
that the TreeWalk main loop knows to enter into the subtree iterator.
However the subtree iterator needs to only have entry "b.txt" within
its entry list.
 
Show 5 quoted lines
> I suggest to put all the classes from the package
> org.spearce.jgit.treewalk and the package
> org.spearce.jgit.lib.fileiteration into one package. Please tell me
> which package and I will send a patch, or do it yourself. I don't have
> any outstanding changes.

The treewalk package is already established so I would say add them there. Since you are the original developer and your code is not yet in mainline I would ask that you perform the renames.

> I don't see a easy way of porting my Rules implementation to the
> TreeFilter framework, but as you may noticed it is may not necessary to
> do so.

The TreeFilter framework is perhaps not the right API for ignore rules, that is likely true. It also works with paths as byte[] and not as String, because we get byte[] (generally UTF-8 encoded) data from canonical tree objects when reading from the object database. Avoiding the conversion for most entries is a huge performance improvement for us.

I still won't give up my silly dream for `jgit log -- 'foo/*.c'`, but maybe we do need two different implementations to make things work out well.

-- 
Shawn.

← back to recent threads