The handle_property function is the part of read_props that would be
interesting for most people: semantics of properties rather than the
algorithm for parsing them.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove some newlines from handle_node() that are not needed for
clarity.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Node-action: change is not appropriate when switching between file and
directory or adding a new file. Current svn-fe silently accepts such
nodes and the resulting tree has missing files in the "changed when
meant to add" case.
Node-action: add requires some content (text or directory); there is
no such thing as an "intent to add" node in svn dumps. Current svn-fe
accepts such contentless adds but produces an invalid fast-import
stream that refers to nonexistent mark :0 in response.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It would be better to flag such errors and let the import proceed
anyway, but for now it is simpler not to worry about recovery
from such weird cases.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The mode for each file in an svn-format dump is kept in the properties
section. The properties section is read as soon as possible to allow
the correct mode to be filled in when registering the file with the
repo_tree lib.
To support nodes with a missing properties section, svn-fe determines
the mode in three stages:
- The kind (directory or file) of the node is read from the dump and
used to make an initial estimate (040000 or 100644).
- Properties are read in and allowed to override this for symlinks
and executables.
- If there is no properties section, the mode from the previous
content of the path is left alone, overriding the above
considerations.
This is a bit of a mess, and worse, it would get even more complicated
once we start to support property deltas. If we could only register
the file with a provisional value for mode and then change it later
when properties say so, the procedure would be much simpler.
... oh, right, we can.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are two functions to change the staged content for a path in the
svn importer's active commit: repo_replace, which changes the text and
returns the mode, and repo_modify, which changes the text and mode and
returns nothing.
Worse, there are more subtle differences:
- A mark of 0 passed to repo_modify means "use the existing content".
repo_replace uses it as mark :0 and produces a corrupt stream.
- When passed a path that is not part of the active commit,
repo_replace returns without doing anything. repo_modify
transparently adds a new directory entry.
Get rid of both and introduce a new function with the best features of
both: repo_modify_path modifies the mode, content, or both for a path,
depending on which arguments are zero. If no such dirent already
exists, it does nothing and reports the error by returning 0.
Otherwise, the return value is the resulting mode.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Simplify by reducing the "Node-action: replace" case to "Node-action:
add". This way, the main part of handle_node() only has to deal with
"add" and "change" nodes.
Functional change: replacing a symlink or executable without setting
properties will reset the mode.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Take care of "Node-action: delete" as soon as possible, so we can stop
worrying about that case in the rest of the function.
Functional change: catch deletion nodes with features that would not
apply to them (text, properties, or origin data) and error out for
those cases.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allocate a mark if needed as soon as possible so later code can use
"if (mark)" to check if this node has text attached rather than
explicitly checking for Text-content-length.
While at it, reject directory nodes with text attached; the presence
of such a node would indicate a bug in the dump generator or svn-fe's
understanding. In the long term, it would be nice to be able to
continue parsing and save the error for later, but for now it is
simpler to error out right away.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is possible for a path node in an SVN-format dump file to leave out
the properties section. svn-fe handles this by carrying over the
properties (in particular, file type) from the old version of that
node.
To support this, handle_node tests several times whether a
Prop-content-length field is present. Ancient Subversion actually
leaves out the Prop-content-length field even for nodes with
properties, so that's not quite the right check. Besides, this detail
of mechanism is distracting when the question at hand is instead what
content the new node should have.
So introduce a local have_props variable. The semantics are the same
as before; the adaptations to support ancient streams that leave out
the prop-content-length can wait until someone needs them.
Signed-off-by: Jonathan Nieder <jrnieer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The mark variable is only used in handle_node(). Its life is
very short and simple: first, a new mark number is allocated if
this node has text attached, then that mark is recorded in the
in-core tree being built up, and lastly the mark is communicated
to fast-import in the stream along with the associated text.
A new reader may worry about interaction with other code, especially
since mark is not initialized to zero in handle_node() itself.
Disperse such worries by making it local. No functional change
intended.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The srcRev variable is only used in handle_node(); its purpose
is to hold the old mode for a path, to only be used if properties
are not being changed. Narrow its scope to make its meaningful
lifetime more obvious.
No functional change intended. Add some tests as a sanity-check
for the simplest case (no renames).
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
test-svn-fe segfaults when passed a bogus path. Simplify debugging by
exiting with a meaningful error message instead.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since the dumpfile version 1 days, the Subversion dump format
gained some new fields:
- a unique identifier for the repository (version 2 format)
- whether the text and properties for a node should be
interpreted as deltas
- checksums for a delta's preimage
- SHA-1 sums as alternatives to the existing MD5 checksums for
copy source and the payload (delta).
For now what is relevant to us is the Text-delta and Prop-delta
fields, since not noticing these causes a dump file to be
misinterpreted (see the previous commit).
[jn: with tests]
Signed-off-by: David Barr <david.barr@cordelta.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
By ignoring the Text-Delta and Prop-Delta node fields, current svn-fe
happily mistakes deltas for full text and instead of cleanly erroring
out, it produces a valid but semantically bogus fast-import stream
when fed a dump file in the modern "svnadmin dump --deltas" format.
Dump file parsers are supposed to ignore header fields they don't
understand (to allow for backward-compatible extensions), but they are
also supposed to check the SVN-fs-dump-format-version header to
prevent misinterpretation of non backward-compatible extensions.
Do so.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* kb/maint-submodule-savearg:
submodule: only preserve flags across recursive status/update invocations
submodule: preserve all arguments exactly when recursing
* jm/mailmap:
t4203: do not let "git shortlog" DWIM based on tty
t4203 (mailmap): stop hardcoding commit ids and dates
mailmap: fix use of freed memory
* jk/push-progress:
push: pass --progress down to git-pack-objects
t5523-push-upstream: test progress messages
t5523-push-upstream: add function to ensure fresh upstream repo
test_terminal: ensure redirections work reliably
test_terminal: catch use without TTY prerequisite
test-lib: allow test code to check the list of declared prerequisites
tests: test terminal output to both stdout and stderr
tests: factor out terminal handling from t7006
* sg/completion:
bash: support pretty format aliases
bash: support more 'git notes' subcommands and their options
bash: not all 'git bisect' subcommands make sense when not bisecting
bash: offer refs for 'git bisect start'
* sg/bisect:
bisect: check for mandatory argument of 'bisect replay'
bisect: improve error msg of 'bisect reset' when original HEAD is deleted
bisect: improve error message of 'bisect log' while not bisecting
* jn/gitweb-test:
gitweb/Makefile: Include gitweb/config.mak
gitweb/Makefile: Add 'test' and 'test-installed' targets
t/gitweb-lib.sh: Add support for GITWEB_TEST_INSTALLED
gitweb: Move call to evaluate_git_version after evaluate_gitweb_config
baselen used to be the result of common_prefix() when it was made
builtin. Since 1d8842d (Add 'fill_directory()' helper function for
directory traversal - 2009-05-14), its value will always be
zero. Remove it because it's no longer variable.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The sentence about 'branch.<name>.rebase' refers to the first sentence
in the paragraph and not to the sentence about avoiding rebasing
non-local changes. Clarify this.
Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This may help to understand why --graph causes more comments to
be selected.
Signed-off-by: Yann Dirson <ydirson@altern.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
qname is the result of quote_path_relative(), which does
quote_c_style_counted() internally. Remove the hard-coded quotes.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It's pretty straightforward, but a stripped-down example
never hurts. And we should make clear that it is explicitly
OK to use SIG_DFL and SIG_IGN.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It makes little sense to have --diff-filter in the middle of them, and
even spares an ifndef::git-format-patch.
Signed-off-by: Yann Dirson <ydirson@altern.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change submodule tests that piped to diff(1) to use test_cmp. The
resulting unified diff is easier to read.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Split the "message in editor has initial comment" test into three
tests. The motivation is to be able to only skip the middle part under
NO_GETTEXT_POISON.
In addition the return value of 'git tag' was being returned. We now
check that it's non-zero. I used ! instead of test_must_fail so that
the GIT_EDITOR variable was only used in this command invocation, and
because the surrounding tests use this style.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If any strategy options are passed to -X, the strategy will always be
set to 'recursive'. According to the documentation, it should default to
'recursive' if it is not set, but it should be possible to set it to
other values.
This fixes a regression introduced in v1.7.3-rc0~67^2 (2010-07-29).
Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
