You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Brian Denny <br...@briandenny.net> on 2003/11/08 17:29:34 UTC

[PATCH] Re: All breakage tests failing in svn_io_file_rename

On Fri, Nov 07, 2003 at 07:30:07PM -0800, Brian Denny wrote:
> On Sat, Nov 08, 2003 at 01:10:19AM +0000, Philip Martin wrote:
> > One option would be to make a decision based on whether we are using a
> > read-only access baton or a read-write access baton.

...And here's the patch.  Currently running 'make check'.

-brian



Log:
Resolve issue #1589 -- "svn_io_temp_dir() inappropriate for some wc 
write ops".  In the context of merge (a read/write operation), create 
the temp file in the working directory instead of the system's tempdir. 

* subversion/libsvn_client/repos_diff.c
  (create_empty_file): New parameter HAVE_WRITE_LOCK.  If TRUE, create 
    the temp file in the working directory instead of the system's tempdir. 
  (get_empty_file): Determine whether we have a write lock.
  (apply_textdelta): Determine whether we have a write lock.



Index: subversion/libsvn_client/repos_diff.c
===================================================================
--- subversion/libsvn_client/repos_diff.c	(revision 7674)
+++ subversion/libsvn_client/repos_diff.c	(working copy)
@@ -353,18 +353,30 @@
 }
 
 
-/* Create an empty file, the path to the file is returned in EMPTY_FILE
+/* Create an empty file, the path to the file is returned in EMPTY_FILE.
+ * If HAVE_WRITE_LOCK is true, create the file in the working directory,
+ * otherwise use a system temp dir.
  */
 static svn_error_t *
 create_empty_file (const char **empty_file,
+                   svn_boolean_t have_write_lock,
                    apr_pool_t *pool)
 {
   apr_file_t *file;
-  const char *temp_dir;
+  const char *temp_path;
 
-  SVN_ERR (svn_io_temp_dir (&temp_dir, pool));
-  SVN_ERR (svn_io_open_unique_file (&file, empty_file, 
-                                    svn_path_join (temp_dir, "tmp", pool),
+  if (have_write_lock)
+    {
+      temp_path = "tmp";
+    }
+  else 
+    {
+      const char *temp_dir;
+      SVN_ERR (svn_io_temp_dir (&temp_dir, pool));
+      temp_path = svn_path_join (temp_dir, "tmp", pool);
+    }
+
+  SVN_ERR (svn_io_open_unique_file (&file, empty_file, temp_path,
                                     "", FALSE, pool));
   SVN_ERR (svn_io_file_close (file, pool));
 
@@ -432,7 +444,9 @@
   /* Create the file if it does not exist */
   if (!b->empty_file)
     {
-      SVN_ERR (create_empty_file (&(b->empty_file), b->pool));
+      svn_boolean_t have_lock;
+      have_lock = (b->adm_access && svn_wc_adm_locked (b->adm_access));
+      SVN_ERR (create_empty_file (&(b->empty_file), have_lock, b->pool));
 
       /* Install a pool cleanup handler to delete the file */
       SVN_ERR (temp_file_cleanup_register (b->empty_file, b->pool));
@@ -693,6 +707,7 @@
                  void **handler_baton)
 {
   struct file_baton *b = file_baton;
+  svn_boolean_t have_lock;
 
   /* Open the file to be used as the base for second revision */
   SVN_ERR (svn_io_file_open (&(b->file_start_revision),
@@ -701,7 +716,9 @@
 
   /* Open the file that will become the second revision after applying the
      text delta, it starts empty */
-  SVN_ERR (create_empty_file (&(b->path_end_revision), b->pool));
+  have_lock = (b->edit_baton->adm_access 
+               && svn_wc_adm_locked (b->edit_baton->adm_access));
+  SVN_ERR (create_empty_file (&(b->path_end_revision), have_lock, b->pool));
   SVN_ERR (temp_file_cleanup_register (b->path_end_revision, b->pool));
   SVN_ERR (svn_io_file_open (&(b->file_end_revision), b->path_end_revision,
                              APR_WRITE, APR_OS_DEFAULT, b->pool));


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

Re: [PATCH] Re: All breakage tests failing in svn_io_file_rename

Posted by Brian Denny <br...@briandenny.net>.
On Sat, Nov 08, 2003 at 09:29:34AM -0800, Brian Denny wrote:
> 
> ...And here's the patch.  Currently running 'make check'.

Committed in r7704.

-brian


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

Re: [PATCH] Re: All breakage tests failing in svn_io_file_rename

Posted by Brian Denny <br...@briandenny.net>.
On Tue, Nov 11, 2003 at 06:07:21AM +0200, Jani Averbach wrote:
> On 2003-11-10 19:42-0800, Brian Denny wrote:
> > Do you get these failures without my patch as well?  I am pretty sure 
> > that I saw an export-tests.py failure both with and without my patch.
> > 
> 
> Yes, here is result for ra_svn with pure r7695:
> FAIL:  export_tests.py 6: export with keyword translation

Cool.  I will commit this patch later today unless I receive objections
before then.

-brian


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

Re: [PATCH] Re: All breakage tests failing in svn_io_file_rename

Posted by Jani Averbach <ja...@cc.jyu.fi>.
On 2003-11-10 19:42-0800, Brian Denny wrote:
> On Tue, Nov 11, 2003 at 03:33:33AM +0200, Jani Averbach wrote:
> >
> > ra_local: all test passed
> > ra_svn: 1 failed, export_tests.py 6: export with keyword translation
> > ra_dav: 1 failed, export_tests.py 6: export with keyword translation
> 
> Do you get these failures without my patch as well?  I am pretty sure 
> that I saw an export-tests.py failure both with and without my patch.
> 

Yes, here is result for ra_svn with pure r7695:
FAIL:  export_tests.py 6: export with keyword translation
FAIL:  merge_tests.py 2: merge and add new files/dirs with history
FAIL:  merge_tests.py 8: merging similar trees ancestrally unrelated
FAIL:  merge_tests.py 10: merge change into unchanged binary file
FAIL:  merge_tests.py 12: diff after merge that creates a new file
FAIL:  merge_tests.py 13: merge should skip over unversioned obstructions

So those are nothing new:
http://subversion.tigris.org/servlets/ReadMsg?list=svn-breakage&msgNo=5498

BR, Jani

-- 
Jani Averbach


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

Re: [PATCH] Re: All breakage tests failing in svn_io_file_rename

Posted by Brian Denny <br...@briandenny.net>.
On Tue, Nov 11, 2003 at 03:33:33AM +0200, Jani Averbach wrote:
> On 2003-11-10 14:02-0600, kfogel@collab.net wrote:
> > 
> > Any news?
> 
> I ran the test set with shared build (ra_local, ra_svn, ra_dav) and here
> is the report:
> 
> ra_local: all test passed
> ra_svn: 1 failed, export_tests.py 6: export with keyword translation
> ra_dav: 1 failed, export_tests.py 6: export with keyword translation

Do you get these failures without my patch as well?  I am pretty sure 
that I saw an export-tests.py failure both with and without my patch.

> 
> Without the patch, the merge tests will fail.

That's what I get too.  merge-test.py 2 failed for me without my patch,
and succeeded with it.  

-brian


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

Re: [PATCH] Re: All breakage tests failing in svn_io_file_rename

Posted by kf...@collab.net.
Jani Averbach <ja...@cc.jyu.fi> writes:
> ra_svn: 1 failed, export_tests.py 6: export with keyword translation
> ra_dav: 1 failed, export_tests.py 6: export with keyword translation

These are real FAILs, right, not XFAILs?

I thought John Szakmeister knows this test fails, so it should be set
to XFAIL, but it's not... John, am I misunderstanding, or should we
make export_keyword_translation() XFAIL?

-K

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

Re: [PATCH] Re: All breakage tests failing in svn_io_file_rename

Posted by Jani Averbach <ja...@cc.jyu.fi>.
On 2003-11-10 14:02-0600, kfogel@collab.net wrote:
> Brian Denny <br...@briandenny.net> writes:
> > > On Sat, Nov 08, 2003 at 01:10:19AM +0000, Philip Martin wrote:
> > > > One option would be to make a decision based on whether we are using a
> > > > read-only access baton or a read-write access baton.
> > 
> > ...And here's the patch.  Currently running 'make check'.
> 
> Any news?

I ran the test set with shared build (ra_local, ra_svn, ra_dav) and here
is the report:

ra_local: all test passed
ra_svn: 1 failed, export_tests.py 6: export with keyword translation
ra_dav: 1 failed, export_tests.py 6: export with keyword translation

Without the patch, the merge tests will fail.

Here is the used patch, I had to apply Bryan's patch by hand, because
it was out of date. So the included patch is the Brian's patch, and
hopefully it is exactly same as original (it should be).

BR, Jani

Brian wrote:

Log: 
 Resolve issue #1589 -- "svn_io_temp_dir() inappropriate for some wc 
 write ops". In the context of merge (a read/write operation), create 
 the temp file in the working directory instead of the system's tempdir. 
 

* subversion/libsvn_client/repos_diff.c 
   (create_empty_file): New parameter HAVE_WRITE_LOCK. If TRUE, create 
     the temp file in the working directory instead of the system's tempdir. 
   (get_empty_file): Determine whether we have a write lock. 
   (apply_textdelta): Determine whether we have a write lock. 
 

Index: svn/subversion/libsvn_client/repos_diff.c
===================================================================
--- svn/subversion/libsvn_client/repos_diff.c	(revision 7693)
+++ svn/subversion/libsvn_client/repos_diff.c	(working copy)
@@ -353,19 +353,31 @@
 }
 
 
-/* Create an empty file, the path to the file is returned in EMPTY_FILE
+/* Create an empty file, the path to the file is returned in EMPTY_FILE.
+ * If HAVE_WRITE_LOCK is true, create the file in the working directory,
+ * otherwise use a system temp dir.
  */
 static svn_error_t *
 create_empty_file (const char **empty_file,
+                   svn_boolean_t have_write_lock,
                    apr_pool_t *pool)
 {
   apr_file_t *file;
-  const char *temp_dir;
+  const char *temp_path;
 
-  SVN_ERR (svn_io_temp_dir (&temp_dir, pool));
-  SVN_ERR (svn_io_open_unique_file (&file, empty_file, 
-                                    svn_path_join (temp_dir, "tmp", pool),
-                                    "", FALSE, pool));
+  if (have_write_lock)
+    {
+      temp_path = "tmp";
+    }
+  else
+    {
+      const char *temp_dir;
+      SVN_ERR (svn_io_temp_dir (&temp_dir, pool));
+      temp_path = svn_path_join (temp_dir, "tmp", pool);
+    }
+
+  SVN_ERR (svn_io_open_unique_file (&file, empty_file, temp_path,
+                                     "", FALSE, pool));
   SVN_ERR (svn_io_file_close (file, pool));
 
   return SVN_NO_ERROR;
@@ -432,7 +444,9 @@
   /* Create the file if it does not exist */
   if (!b->empty_file)
     {
-      SVN_ERR (create_empty_file (&(b->empty_file), b->pool));
+      svn_boolean_t have_lock;
+      have_lock = (b->adm_access && svn_wc_adm_locked (b->adm_access));
+      SVN_ERR (create_empty_file (&(b->empty_file), have_lock, b->pool));
 
       /* Install a pool cleanup handler to delete the file */
       SVN_ERR (temp_file_cleanup_register (b->empty_file, b->pool));
@@ -693,6 +707,7 @@
                  void **handler_baton)
 {
   struct file_baton *b = file_baton;
+  svn_boolean_t have_lock;
 
   /* Open the file to be used as the base for second revision */
   SVN_ERR (svn_io_file_open (&(b->file_start_revision),
@@ -701,7 +716,9 @@
 
   /* Open the file that will become the second revision after applying the
      text delta, it starts empty */
-  SVN_ERR (create_empty_file (&(b->path_end_revision), b->pool));
+  have_lock = (b->edit_baton->adm_access
+               && svn_wc_adm_locked (b->edit_baton->adm_access));
+  SVN_ERR (create_empty_file (&(b->path_end_revision), have_lock, b->pool));
   SVN_ERR (temp_file_cleanup_register (b->path_end_revision, b->pool));
   SVN_ERR (svn_io_file_open (&(b->file_end_revision), b->path_end_revision,
                              APR_WRITE, APR_OS_DEFAULT, b->pool));


-- 
Jani Averbach

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

Re: [PATCH] Re: All breakage tests failing in svn_io_file_rename

Posted by kf...@collab.net.
Brian Denny <br...@briandenny.net> writes:
> > On Sat, Nov 08, 2003 at 01:10:19AM +0000, Philip Martin wrote:
> > > One option would be to make a decision based on whether we are using a
> > > read-only access baton or a read-write access baton.
> 
> ...And here's the patch.  Currently running 'make check'.

Any news?

(I was about to enter something in issue #1589, but then I realized I
didn't know what to write :-) ).

Btw, the 'make check' results will be more meaningful if we know that
svn_io_temp_dir() returns a directory on a different device from your
working copy.  Then those merge tests would only pass if you had fixed
the bug.  (If everything's on the same device, then they'd pass
whether the bug was fixed or not.)

-Karl

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