On Fri, 8 Aug 2003, Sander Striker wrote:
> > From: Shlomi Fish [mailto:shlomif@vipe.technion.ac.il]
> > Sent: Thursday, August 07, 2003 8:21 PM
>
> > OK. The updated patch (without the tests) is here:
>
> Hi Shlomi,
>
> You've been a real sport in providing us iterations of patches.
> I've reviewed your patch and there are some issues. If you
> would like to do another patch, that would be great.
Sure! Just give me some time to work on it.
> If not,
> let me know (I'll handle it myself then).
>
> Anyhow, comments in line.
>
> Thanks,
>
> Sander
>
>
> > http://t2.technion.ac.il/~shlomif/svn-patches/issue-1297-fix-rev4.patch.txt
>
> Index: subversion/libsvn_wc/update_editor.c
> ===================================================================
> --- subversion/libsvn_wc/update_editor.c (revision 6665)
> +++ subversion/libsvn_wc/update_editor.c (working copy)
> @@ -2307,3 +2307,135 @@
>
> return SVN_NO_ERROR;
> }
> +
> +svn_error_t *
> +svn_wc_put_repos_file_in_wc (const char *dst_path,
> + svn_wc_adm_access_t *adm_access,
> + svn_wc_put_repos_file_in_wc_helper helper,
>
> This would be the really long helper function name. What about:
> svn_wc_add_repost_file[_helper] ?
OK.
>
> + void *helper_baton,
> + apr_pool_t *pool
> + )
> +{
> + const char *text_base_path;
> + svn_stream_t *fstream;
> + apr_file_t *fp;
> + apr_hash_t *props;
> + const char *prop_base_path, *prop_path;
> + svn_stringbuf_t *log_accum;
> + apr_file_t *log_fp = NULL;
> + const char *parent_dir;
> + const char *base_name;
> + apr_status_t status;
> + const char * tmptext;
> + apr_hash_index_t *hi;
> + svn_node_kind_t kind;
> + apr_hash_t *regular_props;
> +
> + text_base_path = svn_wc__text_base_path(dst_path, FALSE, pool);
> +
> + SVN_ERR_W (svn_io_file_open (&fp, text_base_path,
> + (APR_CREATE | APR_WRITE),
> + APR_OS_DEFAULT, pool),
> + "failed to open file for writing.");
> +
> + /* Create a generic stream that operates on this file. */
> + fstream = svn_stream_from_aprfile (fp, pool);
>
> Should be a temporary file.
>
OK. Should I then pass TRUE to svn_wc__text_base_path() as tmp?
> + SVN_ERR (helper (fstream, &props, helper_baton));
> +
> + /* Close the file. */
> + status = apr_file_close (fp);
> + if (status)
> + return svn_error_createf (status, NULL,
> + "failed to close file '%s'.",
> + dst_path);
> +
> + /* Write the props and prop-base file for this file */
> +
> + regular_props = apr_hash_make (pool);
> + if (props)
> + {
> + for (hi = apr_hash_first(pool, props); hi; hi = apr_hash_next(hi))
> + {
> + const void *key;
> + void *val;
> + enum svn_prop_kind prop_kind;
> +
> + apr_hash_this (hi, &key, NULL, &val);
> +
> + /* We only want to set 'normal' props. For now, we're
> + ignoring any wc props (they're not needed when we commit
> + an addition), and we're ignoring entry props (they're
> + written to the entries file as part of the post-commit
> + processing). */
> + prop_kind = svn_property_kind (NULL, key);
> + if (prop_kind == svn_prop_regular_kind)
> + {
> + apr_hash_set (regular_props, key, APR_HASH_KEY_STRING, val);
> + }
> + }
> + }
> +
> + SVN_ERR (svn_wc__prop_path (&prop_path, dst_path, adm_access, FALSE, pool));
> +
> + SVN_ERR (svn_wc__prop_base_path (&prop_base_path, dst_path, adm_access,
> + FALSE, pool));
> +
> + svn_wc__save_prop_file(prop_path, regular_props, pool);
> + svn_wc__save_prop_file(prop_base_path, regular_props, pool);
>
> You need to save these to temporary files.
>
OK. Again, I can use TRUE as tmp.
> + /* Now copy the text-base to the main file while applying the end-of-line
> + * and keywords policies */
> +
> + /* Split FILE_PATH. */
> + svn_path_split (dst_path, &parent_dir, &base_name, pool);
> +
> +
> + /* Open a log file. This is safe because the adm area is locked
> + right now. */
> + SVN_ERR (svn_wc__open_adm_file (&log_fp,
> + parent_dir,
> + SVN_WC__ADM_LOG,
> + (APR_WRITE | APR_CREATE), /* not excl */
> + pool));
> +
> + log_accum = svn_stringbuf_create ("", pool);
> +
> + tmptext = svn_wc__text_base_path(base_name, FALSE, pool);
>
> Why do you calculate the text base path again here? You already have it
> in text_base_path.
>
Hmmm... OK.
> Here you move the text-base temporary file like so:
>
> svn_xml_make_open_tag (&log_accum,
> pool,
> svn_xml_self_closing,
> SVN_WC__LOG_MV,
> SVN_WC__LOG_ATTR_NAME, tmp_text_base,
> SVN_WC__LOG_ATTR_DEST, real_text_base,
> NULL);
>
> Then you make it read only like so:
>
> svn_xml_make_open_tag (&log_accum, pool,
> svn_xml_self_closing,
> SVN_WC__LOG_READONLY,
> SVN_WC__LOG_ATTR_NAME, real_text_base,
> NULL);
>
> And the same for the props (move tmp to real, then make read only).
>
OK. Will be done.
> + svn_xml_make_open_tag (&log_accum, pool,
> + svn_xml_self_closing,
> + SVN_WC__LOG_CP_AND_TRANSLATE,
> + SVN_WC__LOG_ATTR_NAME, tmptext,
>
> This would become:
> + SVN_WC__LOG_ATTR_NAME, real_text_base,
>
>
Right.
> + SVN_WC__LOG_ATTR_DEST, base_name,
> + NULL);
>
> [...]
> + /* Make the text-base, props and prop-base files read-only */
> +
> + SVN_ERR (svn_io_set_file_read_only (prop_path, TRUE, pool));
> + SVN_ERR (svn_io_set_file_read_only (prop_base_path, TRUE, pool));
> + /*
> + * The text_base_path must exist and so it should fail if it doesn't.
> + * */
> + SVN_ERR (svn_io_set_file_read_only (text_base_path, FALSE, pool));
>
> Don't need this, since it was all done by running the log.
>
OK.
Hmmm... it seems I did not completely grok the way to do things in
libsvn_wc. I'll fix it according to your input and send a new patch.
Regards,
Shlomi Fish
> + return SVN_NO_ERROR;
> +}
>
----------------------------------------------------------------------
Shlomi Fish shlomif@vipe.technion.ac.il
Home Page: http://t2.technion.ac.il/~shlomif/
An apple a day will keep a doctor away. Two apples a day will keep two
doctors away.
Falk Fish
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Aug 8 09:28:27 2003