Adding empty files: change to svn_fs_make_file and documentation for the delta editor?

So Ben brought up a bizarre bug on #svn-dev today. A user was using
svnsync to copy his project to Google Code, and zero-length files were
not being created on the destination.

We eventually tracked down the bug. In the repository, each revision
has a list of "changed-path" information (svn_fs_path_change_t),
describing for each changed node rev what kind of change it was,
including whether or not text has been modified. It is unclear from
the documentation whether or not a new *empty* file created without
history should have this flag set to true. However, empty files that
are created with "svn commit" definitely will have the text_mod flag
set to true. Additionally, empty files created via "svnadmin load"
will have the text_mod flag set to true.

For whatever reason, in the user's repository, the text_mod flag was
false on a new uncopied zero-length file. This led svn_repos_replay
to decide not to call apply_textdelta on it, which leads to ra_neon
and ra_serf not issuing a PUT command for that file, which leads to
the file not being created.

As far as I can tell, there's no way using the Subversion codebase
itself to create the revision that the user did (we don't know yet if
he used some external tool). In my opinion, the bug is that the
revision exists with a "false" text_mod. This can be fixed by
patching the implementations of svn_fs_make_file in both filesystems
to set text_mod to true for newly-created (non-copied) file nodes. I
think we should do this. This should guarantee that new empty files
(created through commits, loads, or direct svn_fs_* calls) always have
text_mod set to TRUE.

Additionally, given that the neon and serf commit editor
implementations assume that apply_textdelta will be called on new
empty files (which the libsvn_client commit driver always does, as
will the repos_replay commit driver if the revision isn't broken as
above in the first place), we should change the documentation of the
delta editor to explicitly state that apply_textdelta is required for
all new (uncopied) files.

I propose the attached two patches. What do you think?

[[[
The DAV-based RA commit editors have always assumed that
apply_textdelta will be called on all copied files, even empty ones,
and the client commit driver has always done so; document this
as an explicit requirement for driving a delta editor.

* subversion/include/svn_delta.h
  (add_file): Explicitly state that you need to call apply_textdelta
   for new uncopied files.
]]]

[[[
Ensure that new uncopied files always have the text-modification flag
set, even if they are empty.

Previously, empty files created with "svn commit" or "svnadmin load"
would always have the text-modification flag set, but direct usage of
the svn_fs_make_file API would leave the flag unset; this led to
issues such as svn_repos_replay2 not sending an apply_textdelta for
them.

* subversion/libsvn_fs_base/tree.c
  (txn_body_make_file): Pass true value for text-modifications flag in
   the call to add_change.

* subversion/libsvn_fs_fs/tree.c
  (fs_make_file): Pass true value for text-modifications flag in
   the call to add_change.
]]]

-- 
David Glasser | glasser_at_mit.edu | http://www.davidglasser.net/

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