There are two short answers to your question: 1) we don't have a good handle on what to do with file externals in wc-ng (we temporarily punted) 2) per the doc in BASE_NODE, file externals are just a copy of an existing property, so we shouldn't need to duplicate the values into another column
I'd be interested in asking why a separate column is needed. Can the file externals code be rearranged to more directly interpret svn:externals as needed, rather than referring to custom metadata? Cheers, -g On Thu, Mar 18, 2010 at 16:52, C. Michael Pilato <[email protected]> wrote: > I started yesterday trying to convert a bit of svn_wc_entry_t-wielding code > related to file externals into WC-NGness. I thought I had identified an > easy abstraction of some related code, so I made that abstraction and found > that it was a miserable failure. > > I'm attaching two patches. The first (file-externals-abstraction-patch.txt) > just carves some code out of the entry-writing logic and drops it into > wc_db.[ch]. This patch works fine. > > The second (file-externals-definition-patch.txt) was where I thought, "Hey, > I can re-use the code that sets the file_external_* bits of the entry here, > too!" I was wrong. And though it took me far too long to do so, I think I > finally realize why I was wrong. > > See, in subversion/libsvn_client/externals.c:switch_file_external(), there > is logic around file external handling that basically looks like this: > > - create a file scheduled for addition (using svn_wc_add4) > - set the magic file_external_* bits on that file's entry > (svn_wc__set_file_external_location) > - switch the file to the external location (svn_client__switch_internal) > > In the current codebase, that works fine because that second step is using > svn_wc__modify_entry2() which actually creates a BASE_NODE row for the new > file with the file_external_* bits set. But I think this is a bit of a > stretch in terms of legitimacy. Until the switch happens, this is just a > new file scheduled for addition, right? There shouldn't be a base node for > that object at all. > > My patched code doesn't work because there's not yet a base node for the new > file, so attempts to update the file_external_* data on that base node > silently fail. > > Clearly the code wants to hang this file external information somewhere. > The WORKING tree lacks schema support for it. The switch logic needs it to > make decisions about what it's doing, but it seems wrong to pass this bit of > hackery through the public update/switch API. I hesitate to hack in yet > another "private" workaround API. > > Anybody got any bright ideas or precedent that I can refer to? > > -- > C. Michael Pilato <[email protected]> > CollabNet <> www.collab.net <> Distributed Development On Demand > > Index: subversion/libsvn_wc/entries.c > =================================================================== > --- subversion/libsvn_wc/entries.c (revision 924936) > +++ subversion/libsvn_wc/entries.c (working copy) > @@ -2103,22 +2103,11 @@ > ### This is a hack! */ > if (entry->file_external_path) > { > - svn_sqlite__stmt_t *stmt; > - const char *str; > - > - SVN_ERR(svn_wc__serialize_file_external(&str, > + SVN_ERR(svn_wc__db_op_set_file_external(db, entry_abspath, > entry->file_external_path, > > &entry->file_external_peg_rev, > &entry->file_external_rev, > scratch_pool)); > - > - SVN_ERR(svn_sqlite__get_statement(&stmt, sdb, > - STMT_UPDATE_FILE_EXTERNAL)); > - SVN_ERR(svn_sqlite__bindf(stmt, "iss", > - (apr_uint64_t)1 /* wc_id */, > - entry->name, > - str)); > - SVN_ERR(svn_sqlite__step_done(stmt)); > } > } > > Index: subversion/libsvn_wc/wc_db.c > =================================================================== > --- subversion/libsvn_wc/wc_db.c (revision 924936) > +++ subversion/libsvn_wc/wc_db.c (working copy) > @@ -2957,6 +2957,43 @@ > > > svn_error_t * > +svn_wc__db_op_set_file_external(svn_wc__db_t *db, > + const char *local_abspath, > + const char *external_path, > + const svn_opt_revision_t *external_peg_rev, > + const svn_opt_revision_t *external_rev, > + apr_pool_t *scratch_pool) > +{ > + svn_wc__db_pdh_t *pdh; > + const char *local_relpath; > + svn_sqlite__stmt_t *stmt; > + const char *str; > + > + SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath)); > + SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath, > + svn_sqlite__mode_readwrite, > + scratch_pool, scratch_pool)); > + VERIFY_USABLE_PDH(pdh); > + > + SVN_ERR(svn_wc__serialize_file_external(&str, external_path, > + external_peg_rev, external_rev, > + scratch_pool)); > + > + SVN_ERR(svn_sqlite__get_statement(&stmt, pdh->wcroot->sdb, > + STMT_UPDATE_FILE_EXTERNAL)); > + SVN_ERR(svn_sqlite__bindf(stmt, "iss", > + pdh->wcroot->wc_id, > + svn_relpath_basename(local_relpath, > scratch_pool), > + str)); > + SVN_ERR(svn_sqlite__step_done(stmt)); > + > + flush_entries(pdh); > + > + return SVN_NO_ERROR; > +} > + > + > +svn_error_t * > svn_wc__db_op_revert(svn_wc__db_t *db, > const char *local_abspath, > svn_depth_t depth, > Index: subversion/libsvn_wc/wc_db.h > =================================================================== > --- subversion/libsvn_wc/wc_db.h (revision 924936) > +++ subversion/libsvn_wc/wc_db.h (working copy) > @@ -1073,6 +1073,20 @@ > apr_pool_t *scratch_pool); > > > +/** Set the file external definition associated with LOCAL_ABSPATH in > + * DB. The definition consists of the EXTERNAL_PATH, EXTERNAL_PEG_REV > + * revision, and EXTERNAL_REV working revision specification. > + * > + * Use SCRATCH_POOL for any temporary allocations. > + */ > +svn_error_t * > +svn_wc__db_op_set_file_external(svn_wc__db_t *db, > + const char *local_abspath, > + const char *external_path, > + const svn_opt_revision_t *external_peg_rev, > + const svn_opt_revision_t *external_rev, > + apr_pool_t *scratch_pool); > + > /* ### status */ > > > > Index: subversion/libsvn_wc/adm_ops.c > =================================================================== > --- subversion/libsvn_wc/adm_ops.c (revision 924379) > +++ subversion/libsvn_wc/adm_ops.c (working copy) > @@ -2655,29 +2655,28 @@ > const char *repos_root_url, > apr_pool_t *scratch_pool) > { > - svn_wc_entry_t entry = { 0 }; > + const char *external_path; > + svn_opt_revision_t external_peg_rev, external_rev; > > if (url) > { > /* A repository root relative path is stored in the entry. */ > SVN_ERR_ASSERT(peg_rev); > SVN_ERR_ASSERT(rev); > - entry.file_external_path = url + strlen(repos_root_url); > - entry.file_external_peg_rev = *peg_rev; > - entry.file_external_rev = *rev; > + external_path = url + strlen(repos_root_url); > + external_peg_rev = *peg_rev; > + external_rev = *rev; > } > else > { > - entry.file_external_path = NULL; > - entry.file_external_peg_rev.kind = svn_opt_revision_unspecified; > - entry.file_external_rev.kind = svn_opt_revision_unspecified; > + external_path = NULL; > + external_peg_rev.kind = svn_opt_revision_unspecified; > + external_rev.kind = svn_opt_revision_unspecified; > } > > - SVN_ERR(svn_wc__entry_modify2(wc_ctx->db, local_abspath, > - svn_node_unknown, FALSE, > - &entry, SVN_WC__ENTRY_MODIFY_FILE_EXTERNAL, > - scratch_pool)); > - > + SVN_ERR(svn_wc__db_op_set_file_external(wc_ctx->db, local_abspath, > + external_path, &external_peg_rev, > + &external_rev, scratch_pool)); > return SVN_NO_ERROR; > } > > >
