On 10/7/05, Ivan Zhakov <chemodax@gmail.com> wrote:
> On 10/7/05, Erik Huelsmann <ehuels@gmail.com> wrote:
> > > +
> > > + /* Copy pristine text-base to temporary location. */
> > > + SVN_ERR (svn_io_copy_file (src_txtb, tmp_txtb, TRUE, pool));
> > > +
> > > + /* Load source base props. */
> > > + base_props = apr_hash_make (pool);
> > > + SVN_ERR (svn_wc__load_prop_file (src_bprop, base_props, pool));
> > > +
> > > + /* Load source working props. */
> > > + props = apr_hash_make (pool);
> > > + SVN_ERR (svn_wc__load_prop_file (src_wprop, props, pool));
> > > +
> > > + SVN_ERR (svn_wc_add_repos_file2 (dst_path, dst_parent,
> > > + tmp_txtb, src_path,
> > > + base_props, props,
> > > + copyfrom_url, copyfrom_rev, pool));
> >
> > There's a bug here: svn_wc_add_repos_file2() will take ownership of
> > 'src_path', which probably means it needs to be copied to a temporary
> > file before it can be passed to ..._add_repos_file2().
> I didn't see problem here, because I have make .._add_repos_file2()
> copy src_path to temporary file:
Hmm, ok, but the docstring says svn_wc_add_repos_file2 will claim
ownership, which doesn't correspond to the actual implementation now
then.
> > > --- branches/wc-replacements/subversion/libsvn_wc/update_editor.c (original)
> > > +++ branches/wc-replacements/subversion/libsvn_wc/update_editor.c Wed Oct 5 > > + /* Copy new text to temporary file in adm_access. */
> > > + SVN_ERR (svn_wc_create_tmp_file (&file, adm_path, FALSE, pool));
> > > + apr_file_name_get (&tmp_text_path, file);
> > > + SVN_ERR (svn_io_file_close (file, pool));
> > > + SVN_ERR (svn_io_copy_file (new_text_path, tmp_text_path, FALSE, pool));
>
> But you raise question that I also meditate: should
> .._add_repos_file2() should take file in temporary location or
> ..._add_repos_file2() should copy it himself? Now install_file()
> (internal of .._add_repos_file()) support both syntax and check if
> file already in temporary dir. For it is mess. I consider to specify
> one syntax for _add_repos_file() and decide who cares about copy files
> into temporary dir.
I think it should be normal for svn_wc_add_repos_file2() to claim
ownership of the new file: the file needs to be added. It would be
double work if adding the file would result in copying and afterward
deleting the original.
install_file() afaict always claims ownership: if the file is not in
the right path, it *moves* the file to the right path meaning that it
has claimed ownership in those cases too.
bye,
Erik.
Received on Fri Oct 7 20:55:29 2005