Re: Ruby binding test of fs failing on windows

Hi,

On 1/16/07, Kouhei Sutou <kou@cozmixng.org> wrote:
> Hi,
>
> In <ae6cb1100701151651y12503867g60305cb14db929a5@mail.gmail.com>
> "Ruby binding test of fs failing on windows" on Mon, 15 Jan 2007 16:51:47 -0800,
> "Joe Swatosh" <joe.swatosh@gmail.com> wrote:
>
> > in http://thread.gmane.org/gmane.comp.version-control.subversion.devel/84157/focus=84157
> > Brane, Vlad and I conclude that this (taken from test_fs.rb) is
> > probably not a good test:
>
> I'm sorry. I missed the thread.

No problem, it wasn't really Ruby specific so I'm not surprised it
slipped under your radar.

>
> > Interface changes:
> > - Add a close method to Svn::Fs (that would release the new pool
> > member added to Svn::Fs which would close the filesystem).
> > - Add a block form to Svn::Fs.open, Svn::Fs.new, and Svn::Fs.create
> > (with the obvious implementations).
> >
> > Implementation changes:
> > Here I'm kind of fuzzy. I think that we'd have to add a pool member
> > to Svn::Fs (initialized as a "subpool" of the class variable pool?)
> > and pass it into Svn::Ext::Fs.create or Svn::Ext::Fs.open, which would
> > mean modifying their signatures and implementations to use it instead
> > of the global.
>
> It seems that a block form solution is good. But I can't
> implement this because I don't want to expose pool related
> API to Ruby side. I just removed
> Svn::Fs::FileSystem.delete. Could you give me a time to
> think about the implementation?

Of course. I was thinking that the pool stuff would have leak into
the implementation of Svn::Fs since pools would have to be passed to
Svn::Ext::Fs, but that the interface of Svn::Fs would remain unchanged
(except for adding close, and possibly the block versions).
Essentially, my thought is we could remove the @@fs_pool and let each
instance have its own @fs_pool. Then couldn't we also remove the
global that is initialized with the @@fs_pool. (It doesn't *sound*
that hard, but the SWIG stuff is daunting).

I think not exposing the pool related stuff is good. I'm wondering
where you're drawing the line Svn::Fs (where I'm suggesting) or
Svn::Ext::Fs (where it seems to be now).

Getting rid of Svn::Fs::FileSystem.delete might work, but since the
files in the directory are still being held open, even a
FileUtils.rm_rf might fail in this scenario on windows. IOW I'm not
sure that we don't have a deeper issue.

> > There are similar issues with a test in test_repos.rb.
>
> Is the test SvnReposTest#test_create, isn't it? I'll fix it
> soon. And I'll fix SvnFsTest#test_create too.

Yes.

>

Thanks for considering my ideas. You've been very gracious all along
and I greatly appreciate it.

--
Joe Swatosh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org