You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <eh...@gmail.com> on 2007/09/18 11:04:53 UTC

Re: svn commit: r26659 - in trunk/subversion: libsvn_wc tests/cmdline

If you nominate for backport, please add my +1 to it.

On 9/18/07, vgeorgescu@tigris.org <vg...@tigris.org> wrote:
> Author: vgeorgescu
> Date: Tue Sep 18 04:00:46 2007
> New Revision: 26659
>
> Log:
> Fix issue #2928: reverting a scheduled-replace-with-history file doesn't
> restore the original checksum.
>
> * subversion/libsvn_wc/adm_ops.c
>  (revert_admin_things): Restore the original checksum when reverting a
>   replaced with history file.
>
> * subversion/tests/cmdline/revert_tests.py
>  (revert_replaced_with_history_file_1): Rename from
>   revert_replaced_with_history_file, and adjust docstring.
>  (revert_replaced_with_history_file_2): New test.
>  (test_list): Add revert_replaced_with_history_file_2, and rename
>   revert_replaced_with_history_file to revert_replaced_with_history_file_1.
>
>
> Modified:
>   trunk/subversion/libsvn_wc/adm_ops.c
>   trunk/subversion/tests/cmdline/revert_tests.py
>
> Modified: trunk/subversion/libsvn_wc/adm_ops.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/adm_ops.c?pathrev=26659&r1=26658&r2=26659
> ==============================================================================
> --- trunk/subversion/libsvn_wc/adm_ops.c        (original)
> +++ trunk/subversion/libsvn_wc/adm_ops.c        Tue Sep 18 04:00:46 2007
> @@ -1911,6 +1911,19 @@
>           SVN_WC__ENTRY_MODIFY_COPYFROM_URL |
>           SVN_WC__ENTRY_MODIFY_COPYFROM_REV;
>       tmp_entry.copied = FALSE;
> +
> +      /* Reset the checksum if this is a replace-with-history. */
> +      if (tmp_entry.copyfrom_url)
> +        {
> +          const char *revert_base;
> +          unsigned char digest[APR_MD5_DIGESTSIZE];
> +
> +          revert_base = svn_wc__text_revert_path(fullpath, FALSE, pool);
> +          SVN_ERR(svn_io_file_checksum(digest, revert_base, pool));
> +          tmp_entry.checksum = svn_md5_digest_to_cstring(digest, pool);
> +          flags |= SVN_WC__ENTRY_MODIFY_CHECKSUM;
> +        }
> +
>       /* Set this to the empty string, because NULL values will disappear
>          in the XML log file. */
>       tmp_entry.copyfrom_url = "";
>
> Modified: trunk/subversion/tests/cmdline/revert_tests.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/revert_tests.py?pathrev=26659&r1=26658&r2=26659
> ==============================================================================
> --- trunk/subversion/tests/cmdline/revert_tests.py      (original)
> +++ trunk/subversion/tests/cmdline/revert_tests.py      Tue Sep 18 04:00:46 2007
> @@ -629,8 +629,8 @@
>   svntest.actions.run_and_verify_svn(None, expected_output, [], "revert",
>                                      iota_path)
>
> -def revert_replaced_with_history_file(sbox):
> -  "revert a file that was replaced by a copied file"
> +def revert_replaced_with_history_file_1(sbox):
> +  "revert a committed replace-with-history == no-op"
>
>   sbox.build()
>   wc_dir = sbox.wc_dir
> @@ -825,6 +825,36 @@
>   svntest.actions.run_and_verify_svn(None, expected_output, [], "status",
>                                      wc_dir)
>
> +# Test for issue #2928.
> +def revert_replaced_with_history_file_2(sbox):
> +  "reverted replace with history restores checksum"
> +
> +  sbox.build()
> +  wc_dir = sbox.wc_dir
> +  iota_path = os.path.join(wc_dir, 'iota')
> +  mu_path = os.path.join(wc_dir, 'A', 'mu')
> +
> +  # Delete mu and replace it with a copy of iota
> +  svntest.main.run_svn(None, 'rm', mu_path)
> +  svntest.main.run_svn(None, 'cp', iota_path, mu_path)
> +
> +  # Revert mu.
> +  svntest.main.run_svn(None, 'revert', mu_path)
> +
> +  # If we make local mods to the reverted mu the commit will
> +  # fail if the checksum is incorrect.
> +  svntest.main.file_write(mu_path, "new text")
> +  expected_output = svntest.wc.State(wc_dir, {
> +    'A/mu': Item(verb='Sending'),
> +    })
> +  expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
> +  expected_status.tweak('A/mu', status='  ', wc_rev=2)
> +  svntest.actions.run_and_verify_commit(wc_dir,
> +                                        expected_output,
> +                                        expected_status,
> +                                        None, None, None, None, None,
> +                                        wc_dir)
> +
>  ########################################################################
>  # Run the tests
>
> @@ -845,9 +875,10 @@
>               revert_propset__file,
>               revert_propdel__dir,
>               revert_propdel__file,
> -              revert_replaced_with_history_file,
> +              revert_replaced_with_history_file_1,
>               status_of_missing_dir_after_revert,
>               status_of_missing_dir_after_revert_replaced_with_history_dir,
> +              revert_replaced_with_history_file_2,
>              ]
>
>  if __name__ == '__main__':
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
>
>

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

Re: svn commit: r26659 - in trunk/subversion: libsvn_wc tests/cmdline

Posted by Vlad Georgescu <vg...@gmail.com>.
Erik Huelsmann wrote:
> 
> I've slept on the change and I think there's a better one (though that
> one can't be backported): I think we should save the digest in the
> entries file (and thus in a svn_wc_entry_t), extending the entry
> structure with a 'const char *revert_digest'.
> 
> That way, we'll actually know if the file got damaged somehow, instead
> of faking that it's not...

Yep, that would be great.

> Would you have time to implement it this way?

Sure, but probably not today or tomorrow.

-- 
Vlad

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

Re: svn commit: r26659 - in trunk/subversion: libsvn_wc tests/cmdline

Posted by Erik Huelsmann <eh...@gmail.com>.
On 9/18/07, Erik Huelsmann <eh...@gmail.com> wrote:
> If you nominate for backport, please add my +1 to it.
>
> On 9/18/07, vgeorgescu@tigris.org <vg...@tigris.org> wrote:
> > Author: vgeorgescu
> > Date: Tue Sep 18 04:00:46 2007
> > New Revision: 26659
> >
> > Log:
> > Fix issue #2928: reverting a scheduled-replace-with-history file doesn't
> > restore the original checksum.
> >
> > * subversion/libsvn_wc/adm_ops.c
> >  (revert_admin_things): Restore the original checksum when reverting a
> >   replaced with history file.
> >
> > * subversion/tests/cmdline/revert_tests.py
> >  (revert_replaced_with_history_file_1): Rename from
> >   revert_replaced_with_history_file, and adjust docstring.
> >  (revert_replaced_with_history_file_2): New test.
> >  (test_list): Add revert_replaced_with_history_file_2, and rename
> >   revert_replaced_with_history_file to revert_replaced_with_history_file_1.
> >
> >
> > Modified:
> >   trunk/subversion/libsvn_wc/adm_ops.c
> >   trunk/subversion/tests/cmdline/revert_tests.py
> >
> > Modified: trunk/subversion/libsvn_wc/adm_ops.c
> > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/adm_ops.c?pathrev=26659&r1=26658&r2=26659
> > ==============================================================================
> > --- trunk/subversion/libsvn_wc/adm_ops.c        (original)
> > +++ trunk/subversion/libsvn_wc/adm_ops.c        Tue Sep 18 04:00:46 2007
> > @@ -1911,6 +1911,19 @@
> >           SVN_WC__ENTRY_MODIFY_COPYFROM_URL |
> >           SVN_WC__ENTRY_MODIFY_COPYFROM_REV;
> >       tmp_entry.copied = FALSE;
> > +
> > +      /* Reset the checksum if this is a replace-with-history. */
> > +      if (tmp_entry.copyfrom_url)
> > +        {
> > +          const char *revert_base;
> > +          unsigned char digest[APR_MD5_DIGESTSIZE];
> > +
> > +          revert_base = svn_wc__text_revert_path(fullpath, FALSE, pool);
> > +          SVN_ERR(svn_io_file_checksum(digest, revert_base, pool));
> > +          tmp_entry.checksum = svn_md5_digest_to_cstring(digest, pool);
> > +          flags |= SVN_WC__ENTRY_MODIFY_CHECKSUM;
> > +        }
> > +
> >       /* Set this to the empty string, because NULL values will disappear
> >          in the XML log file. */
> >       tmp_entry.copyfrom_url = "";
> >

I've slept on the change and I think there's a better one (though that
one can't be backported): I think we should save the digest in the
entries file (and thus in a svn_wc_entry_t), extending the entry
structure with a 'const char *revert_digest'.

That way, we'll actually know if the file got damaged somehow, instead
of faking that it's not...


Would you have time to implement it this way?

bye,


Erik.

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