Use the function attributes extension to catch mistakes in use of
our own variadic functions that use NULL sentinel at the end
(i.e. like execl(3)) and format strings (i.e. like printf(3)).
* jk/gcc-function-attributes:
Add the LAST_ARG_MUST_BE_NULL macro
wt-status: use "format" function attribute for status_printf
use "sentinel" function attribute for variadic lists
add missing "format" function attributes
For most of our functions that take printf-like formats, we
use gcc's __attribute__((format)) to get compiler warnings
when the functions are misused. Let's give a few more
functions the same protection.
In most cases, the annotations do not uncover any actual
bugs; the only code change needed is that we passed a size_t
to transfer_debug, which expected an int. Since we expect
the passed-in value to be a relatively small buffer size
(and cast a similar value to int directly below), we can
just cast away the problem.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Prior to commit 81d340d4, we did not print any error message
if a remote transport helper died unexpectedly. If a helper
did not print any error message (e.g., because it crashed),
the user could be left confused. That commit tried to
rectify the situation by printing a note that the helper
exited unexpectedly.
However, this makes a much more common case worse: when a
helper does die with a useful message, we print the extra
"Reading from 'git-remote-foo failed" message. This can also
end up confusing users, as they may not even know what
remote helpers are (e.g., the fact that http support comes
through git-remote-https is purely an implementation detail
that most users do not know or care about).
Since we do not have a good way of knowing whether the
helper printed a useful error, and since the common failure
mode is for it to do so, let's default to remaining quiet.
Debuggers can dig further by setting GIT_TRANSPORT_HELPER_DEBUG.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
With "export" remote-helper protocol,
(1) a push that tries to update a remote ref whose name is
different from the pushing side does not work yet, and
(2) the helper may not know how to do --dry-run
Detect such problematic cases and disable them for now.
* fc/transport-helper-no-refspec:
transport-helper: check if the dry-run is supported
transport-helper: barf when user tries old:new
Update transport helper to report errors and maintain ref hierarchy
used to keep track of remote helper state better.
* fc/transport-helper-error-reporting:
transport-helper: fix remote helper namespace regression
test: remote-helper: add missing and
t5801: "VAR=VAL shell_func args" is forbidden
transport-helper: update remote helper namespace
transport-helper: trivial code shuffle
transport-helper: warn when refspec is not used
transport-helper: clarify pushing without refspecs
transport-helper: update refspec documentation
transport-helper: clarify *:* refspec
transport-helper: improve push messages
transport-helper: mention helper name when it dies
transport-helper: report errors properly
Certain remote-helpers (the ones with 'export') would try to push
regardless.
Obviously this is not what the user wants.
Also, add a check for the 'dry-run' option, so remote-helpers can
implement it.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Otherwise with certain remote helpers (the ones that support 'export'),
the users will be pushing to the wrong branch:
git push topic:master
Will push the topic branch, as if the user typed:
git push topic
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 664059f (transport-helper: update remote helper namespace)
updates the namespace when the push succeeds or not; we should do it
only when it succeeded.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* maint:
Git 1.8.2.3
t5004: avoid using tar for checking emptiness of archive
t5004: ignore pax global header file
mergetools/kdiff3: do not use --auto when diffing
transport-helper: trivial style cleanup
If there's already a remote-helper tracking ref, we can fetch the
SHA-1 to report proper push messages (as opposed to always reporting
[new branch]).
The remote-helper currently can specify the old SHA-1 to avoid this
problem, but there's no point in forcing all remote-helpers to be aware
of git commit ids; they should be able to be agnostic of them.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When pushing, the remote namespace is updated correctly
(e.g. refs/origin/master), but not the remote helper's
(e.g. refs/testgit/origin/master), which currently is only
updated while fetching.
Since the remote namespace is used to tell fast-export which commits
to avoid (because they were already imported/exported), it makes
sense to have them in sync so they don't get generated twice. If the
remote helper was implemented properly, they would be ignored, if
not, they probably would end up repeated.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Just shuffle the die() part to make it more explicit, and cleanup the
code-style.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For the modes that need it. In the future we should probably error out,
instead of providing half-assed support.
The reason we want to do this is because if it's not present, the remote
helper might be updating refs/heads/*, or refs/remotes/origin/*,
directly, and in the process fetch will get confused trying to update
refs that are already updated, or older than what they should be. We
shouldn't be messing with the rest of git.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This has never worked, since it's inception the code simply skips all
the refs, essentially telling fast-export to do nothing.
Let's at least tell the user what's going on.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The *:* refspec doesn't work, and never has, clarify the code and
documentation to reflect that. This in effect reverts commit 9e7673e
(gitremote-helpers(1): clarify refspec behaviour).
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This allows a remote helper using the 'export' protocol to specify that
it supports signed tags, changing the handing from 'warn-strip' to
'verbatim'.
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently, attempting to push a signed tag to a remote helper which uses
fast-export results in the remote helper failing because the default
fast-export action for signed tags is "abort". This is not helpful for
users because there is no way to pass additional arguments to
fast-export here, either from the remote helper or from the command
line.
In general, the signature will be invalidated by whatever transformation
a remote helper performs on a tag to push it to a repository in a
different format so the correct behaviour is to strip the tag. Doing
this silently may surprise people, so use "warn-strip" to issue a
warning when a signed tag is encountered.
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Most of these were found using Lucas De Marchi's codespell tool.
Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If there's already a remote-helper tracking ref, we can fetch the SHA-1
to report proper push messages (as opposed to always reporting
[new branch]).
The remote-helper currently can specify the old SHA-1 to avoid this
problem, but there's no point in forcing all remote-helpers to be aware
of git commit ids; they should be able to be agnostic of them.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we try to read from a remote-helper and get EOF or an
error, we print a message indicating that the helper died.
However, users may not know that a remote helper was in use
(e.g., when using git-over-http), or even what a remote
helper is.
Let's print the name of the helper (e.g., "git-remote-https");
this makes it more obvious what the program is for, and
provides a useful token for reporting bugs or searching for
more information (e.g., in manpages).
Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a push fails because the remote-helper died (with fast-export),
the user may not see any error message. We do correctly die with a
failed exit code, as we notice that the helper has died while
reading back the ref status from the helper. However, we don't print
any message. This is OK if the helper itself printed a useful error
message, but we cannot count on that; let's let the user know that
the helper failed.
In the long run, it may make more sense to propagate the error back
up to push, so that it can present the usual status table and give a
nicer message. But this is a much simpler fix that can help
immediately.
While we're adding tests, let's also confirm that the remote-helper
dying is also detected when importing refs. We currently do so
robustly when the helper uses the "done" feature (and that is what
we test). We cannot do so reliably when the helper does not use the
"done" feature, but it is not even worth testing; the right solution
is for the helper to start using "done".
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we push to update an existing ref, if:
* the object at the tip of the remote is not a commit; or
* the object we are pushing is not a commit,
it won't be correct to suggest to fetch, integrate and push again,
as the old and new objects will not "merge". We should explain that
the push must be forced when there is a non-committish object is
involved in such a case.
If we do not have the current object at the tip of the remote, we do
not even know that object, when fetched, is something that can be
merged. In such a case, suggesting to pull first just like
non-fast-forward case may not be technically correct, but in
practice, most such failures are seen when you try to push your work
to a branch without knowing that somebody else already pushed to
update the same branch since you forked, so "pull first" would work
as a suggestion most of the time. And if the object at the tip is
not a commit, "pull first" will fail, without making any permanent
damage. As a side effect, it also makes the error message the user
will get during the next "push" attempt easier to understand, now
the user is aware that a non-commit object is involved.
In these cases, the current code already rejects such a push on the
client end, but we used the same error and advice messages as the
ones used when rejecting a non-fast-forward push, i.e. pull from
there and integrate before pushing again.
Introduce new rejection reasons and reword the messages
appropriately.
[jc: with help by Peff on message details]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
References are allowed to update from one commit-ish to another if the
former is an ancestor of the latter. This behavior is oriented to
branches which are expected to move with commits. Tag references are
expected to be static in a repository, though, thus an update to
something under refs/tags/ should be rejected unless the update is
forced.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
fast-import prints statistics that could be interesting to the
developer of remote helpers.
Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Acked-by: David Michael Barr <b@rr-dav.id.au>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The fast-import commands 'cat-blob' and 'ls' can be used by
remote-helpers to retrieve information about blobs and trees that
already exist in fast-import's memory. This requires a channel from
fast-import to the remote-helper.
remote-helpers that use these features shall advertise the new
'bidi-import' capability to signal that they require the communication
channel. When forking fast-import in transport-helper.c connect it to
a dup of the remote-helper's stdin-pipe. The additional file
descriptor is passed to fast-import via its command line
(--cat-blob-fd). It follows that git and fast-import are connected to
the remote-helpers's stdin.
Because git can send multiple commands to the remote-helper on it's
stdin, it is required that helpers that advertise 'bidi-import' buffer
all input commands until the batch of 'import' commands is ended by a
newline before sending data to fast-import. This is to prevent mixing
commands and fast-import responses on the helper's stdin.
Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Acked-by: David Michael Barr <b@rr-dav.id.au>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The patch below adds a comment to fetch_with_import() explaining the
loop that saves the fetched commit names after 'git fast-import' has
done its work. It avoids some confusion about which refs the
fast-import stream is supposed to use to write its result.
Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The error handling routines add a newline. Remove
the duplicate ones in error messages.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* sp/smart-http-failure-to-push:
: Mask SIGPIPE on the command channel going to a transport helper
disconnect from remote helpers more gently
Conflicts:
transport-helper.c
When git spawns a remote helper program (like git-remote-http),
the last thing we do before closing the pipe to the child
process is to send a blank line, telling the helper that we
are done issuing commands. However, the helper may already
have exited, in which case the parent git process will
receive SIGPIPE and die.
In particular, this can happen with the remote-curl helper
when it encounters errors during a push. The helper reports
individual errors for each ref back to git-push, and then
exits with a non-zero exit code. Depending on the exact
timing of the write, the parent process may or may not
receive SIGPIPE.
This causes intermittent test failure in t5541.8, and is a
side effect of 5238cbf (remote-curl: Fix push status report
when all branches fail). Before that commit, remote-curl
would not send the final blank line to indicate that the
list of status lines was complete; it would just exit,
closing the pipe. The parent git-push would notice the
closed pipe while reading the status report and exit
immediately itself, propagating the failing exit code. But
post-5238cbf, remote-curl completes the status list before
exiting, git-push actually runs to completion, and then it
tries to cleanly disconnect the helper, leading to the
SIGPIPE race above.
This patch drops all error-checking when sending the final
"we are about to hang up" blank line to helpers. There is
nothing useful for the parent process to do about errors at
that point anyway, and certainly failing to send our "we are
done with commands" line to a helper that has already exited
is not a problem.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
read_ref() can (and in test t5800, actually *does*) return NULL.
Don't pass the NULL along to read_ref(). Coincidentally, this mistake
didn't make resolve_ref() blow up, but upcoming changes to
resolve_ref() will make it less forgiving.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remote helpers do not support deleting refs by means of the 'export'
command sincethe fast-import protocol does not support it.
Check explicitly for deleted refs and die early.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that the gitdir location is exported as an environment variable
this can be implemented elegantly without requiring any explicit
flushes nor an ad-hoc exchange of values.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previously the refspec capability could not be listed as
required or their parsing would break.
Most likely the reason the second hunk wasn't caught is because the
series that added 'refspec' as capability, and the one that added
required capabilities were done in parallel.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently the helper must somehow guess how many import statements to
read before it starts outputting its fast-export stream. This is
because the remote helper infrastructure runs fast-import only once,
so the helper is forced to output one stream for all import commands
it will receive. The only reason this worked in the past was because
only one ref was imported at a time.
Change the semantics of the import statement such that it matches
that of the push statement. That is, the import statement is followed
by a series of import statements that are terminated by a '\n'.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Also add check_output from python 2.7.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In other words, use fast-export --use-done-feature to add a 'done'
command at the end of streams passed to remote helpers' "import"
commands, and teach the remote helpers implementing "export" to use
the 'done' command in turn when producing their streams.
The trailing \n in the protocol signals the helper that the
connection is about to close, allowing it to do whatever cleanup
neccesary.
Previously, the connection would already be closed by the
time the trailing \n was to be written. Now that the remote-helper
protocol uses the new done command in its fast-import streams, this
is no longer the case and we can safely write the trailing \n.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previously the status code of all helpers were ignored, allowing
errors that occur to go unnoticed if the error text output by the
helper is not noticed.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The update ref status part of push is useful for the export command
as well, factor it out into it's own function.
Also factor out push_update_ref_status to avoid a long loop without
an explicit condition with a non-trivial body.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The gitdir capability is recognized by git and can be used to tell
the helper where the .git directory is. But it is not mentioned in
the documentation and considered worse than if gitdir was passed
via GIT_DIR environment variable.
Remove support for the gitdir capability and export GIT_DIR instead.
Teach testgit to use env instead of the now-removed gitdir command.
[sr: fixed up documentation]
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we want to push to a remote helper that has the
"export" capability, we collect all of the refs we want to
push and then feed them to fast-export.
However, the list of refs is actually a list of remote refs,
not local refs. The mapped local refs are included via the
peer_ref pointer. So when we add an argument to our
fast-export command line, we must be sure to use the local
peer_ref name (and if there is no local name, it is because
we are not actually sending that ref, or we may not even
have the ref at all).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix warnings from 'make check'.
- These files don't include 'builtin.h' causing sparse to complain that
cmd_* isn't declared:
builtin/clone.c:364, builtin/fetch-pack.c:797,
builtin/fmt-merge-msg.c:34, builtin/hash-object.c:78,
builtin/merge-index.c:69, builtin/merge-recursive.c:22
builtin/merge-tree.c:341, builtin/mktag.c:156, builtin/notes.c:426
builtin/notes.c:822, builtin/pack-redundant.c:596,
builtin/pack-refs.c:10, builtin/patch-id.c:60, builtin/patch-id.c:149,
builtin/remote.c:1512, builtin/remote-ext.c:240,
builtin/remote-fd.c:53, builtin/reset.c:236, builtin/send-pack.c:384,
builtin/unpack-file.c:25, builtin/var.c:75
- These files have symbols which should be marked static since they're
only file scope:
submodule.c:12, diff.c:631, replace_object.c:92, submodule.c:13,
submodule.c:14, trace.c:78, transport.c:195, transport-helper.c:79,
unpack-trees.c:19, url.c:3, url.c:18, url.c:104, url.c:117, url.c:123,
url.c:129, url.c:136, thread-utils.c:21, thread-utils.c:48
- These files redeclare symbols to be different types:
builtin/index-pack.c:210, parse-options.c:564, parse-options.c:571,
usage.c:49, usage.c:58, usage.c:63, usage.c:72
- These files use a literal integer 0 when they really should use a NULL
pointer:
daemon.c:663, fast-import.c:2942, imap-send.c:1072, notes-merge.c:362
While we're in the area, clean up some unused #includes in builtin files
(mostly exec_cmd.h).
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* maint:
Prepare draft release notes to 1.7.4.2
gitweb: highlight: replace tabs with spaces
make_absolute_path: return the input path if it points to our buffer
valgrind: ignore SSE-based strlen invalid reads
diff --submodule: split into bite-sized pieces
cherry: split off function to print output lines
branch: split off function that writes tracking info and commit subject
standardize brace placement in struct definitions
compat: make gcc bswap an inline function
enums: omit trailing comma for portability
Conflicts:
RelNotes
In a struct definitions, unlike functions, the prevailing style is for
the opening brace to go on the same line as the struct name, like so:
struct foo {
int bar;
char *baz;
};
Indeed, grepping for 'struct [a-z_]* {$' yields about 5 times as many
matches as 'struct [a-z_]*$'.
Linus sayeth:
Heretic people all over the world have claimed that this inconsistency
is ... well ... inconsistent, but all right-thinking people know that
(a) K&R are _right_ and (b) K&R are right.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
'bytes' is of type size_t which is unsigned thus can't be negative. But
the assigned write() returns ssize_t, and -1 on error.
For testing < 0, 'bytes' needs to be of a signed type.
Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
