Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Commits on Aug 27, 2014
  1. @peff @gitster

    date: use strbufs in date-formatting functions

    peff authored gitster committed
    Many of the date functions write into fixed-size buffers.
    This is a minor pain, as we have to take special
    precautions, and frequently end up copying the result into a
    strbuf or heap-allocated buffer anyway (for which we
    sometimes use strcpy!).
    
    Let's instead teach parse_date, datestamp, etc to write to a
    strbuf. The obvious downside is that we might need to
    perform a heap allocation where we otherwise would not need
    to. However, it turns out that the only two new allocations
    required are:
    
      1. In test-date.c, where we don't care about efficiency.
    
      2. In determine_author_info, which is not performance
         critical (and where the use of a strbuf will help later
         refactoring).
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Oct 15, 2013
  1. @peff @gitster

    split_ident: parse timestamp from end of line

    peff authored gitster committed
    Split_ident currently parses left to right. Given this
    input:
    
      Your Name <email@example.com> 123456789 -0500\n
    
    We assume the name starts the line and runs until the first
    "<".  That starts the email address, which runs until the
    first ">".  Everything after that is assumed to be the
    timestamp.
    
    This works fine in the normal case, but is easily broken by
    corrupted ident lines that contain an extra ">". Some
    examples seen in the wild are:
    
      1. Name <email>-<> 123456789 -0500\n
    
      2. Name <email> <Name<email>> 123456789 -0500\n
    
      3. Name1 <email1>, Name2 <email2> 123456789 -0500\n
    
    Currently each of these produces some email address (which
    is not necessarily the one the user intended) and end up
    with a NULL date (which is generally interpreted as the
    epoch by "git log" and friends).
    
    But in each case we could get the correct timestamp simply
    by parsing from the right-hand side, looking backwards for
    the final ">", and then reading the timestamp from there.
    
    In general, it's a losing battle to try to automatically
    guess what the user meant with their broken crud. But this
    particular workaround is probably worth doing.  One, it's
    dirt simple, and can't impact non-broken cases. Two, it
    doesn't catch a single breakage we've seen, but rather a
    large class of errors (i.e., any breakage inside the email
    angle brackets may affect the email, but won't spill over
    into the timestamp parsing). And three, the timestamp is
    arguably more valuable to get right, because it can affect
    correctness (e.g., in --until cutoffs).
    
    This patch implements the right-to-left scheme described
    above. We adjust the tests in t4212, which generate a commit
    with such a broken ident, and now gets the timestamp right.
    We also add a test that fsck continues to detect the
    breakage.
    
    For reference, here are pointers to the breakages seen (as
    numbered above):
    
    [1] http://article.gmane.org/gmane.comp.version-control.git/221441
    
    [2] http://article.gmane.org/gmane.comp.version-control.git/222362
    
    [3] http://perl5.git.perl.org/perl.git/commit/13b79730adea97e660de84bbe67f9d7cbe344302
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Sep 20, 2013
  1. @peff @gitster

    format-patch: print in-body "From" only when needed

    peff authored gitster committed
    Commit a908047 taught format-patch the "--from" option,
    which places the author ident into an in-body from header,
    and uses the committer ident in the rfc822 from header.  The
    documentation claims that it will omit the in-body header
    when it is the same as the rfc822 header, but the code never
    implemented that behavior.
    
    This patch completes the feature by comparing the two idents
    and doing nothing when they are the same (this is the same
    as simply omitting the in-body header, as the two are by
    definition indistinguishable in this case). This makes it
    reasonable to turn on "--from" all the time (if it matches
    your particular workflow), rather than only using it when
    exporting other people's patches.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Nov 16, 2012
  1. @peff @gitster

    ident: keep separate "explicit" flags for author and committer

    peff authored gitster committed
    We keep track of whether the user ident was given to us
    explicitly, or if we guessed at it from system parameters
    like username and hostname. However, we kept only a single
    variable. This covers the common cases (because the author
    and committer will usually come from the same explicit
    source), but can miss two cases:
    
      1. GIT_COMMITTER_* is set explicitly, but we fallback for
         GIT_AUTHOR. We claim the ident is explicit, even though
         the author is not.
    
      2. GIT_AUTHOR_* is set and we ask for author ident, but
         not committer ident. We will claim the ident is
         implicit, even though it is explicit.
    
    This patch uses two variables instead of one, updates both
    when we set the "fallback" values, and updates them
    individually when we read from the environment.
    
    Rather than keep user_ident_sufficiently_given as a
    compatibility wrapper, we update the only two callers to
    check the committer_ident, which matches their intent and
    what was happening already.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  2. @peff @gitster

    ident: make user_ident_explicitly_given static

    peff authored gitster committed
    In v1.5.6-rc0~56^2 (2008-05-04) "user_ident_explicitly_given"
    was introduced as a global for communication between config,
    ident, and builtin-commit.  In v1.7.0-rc0~72^2 (2010-01-07)
    readers switched to using the common wrapper
    user_ident_sufficiently_given().  After v1.7.11-rc1~15^2~18
    (2012-05-21), the var is only written in ident.c.
    
    Now we can make it static, which will enable further
    refactoring without worrying about upsetting other code.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on May 25, 2012
  1. @peff @gitster

    ident: reject bogus email addresses with IDENT_STRICT

    peff authored gitster committed
    If we come up with a hostname like "foo.(none)" because the
    user's machine is not fully qualified, we should reject this
    in strict mode (e.g., when we are making a commit object),
    just as we reject an empty gecos username.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  2. @peff @gitster

    ident: rename IDENT_ERROR_ON_NO_NAME to IDENT_STRICT

    peff authored gitster committed
    Callers who ask for ERROR_ON_NO_NAME are not so much
    concerned that the name will be blank (because, after all,
    we will fall back to using the username), but rather it is a
    check to make sure that low-quality identities do not end up
    in things like commit messages or emails (whereas it is OK
    for them to end up in things like reflogs).
    
    When future commits add more quality checks on the identity,
    each of these callers would want to use those checks, too.
    Rather than modify each of them later to add a new flag,
    let's refactor the flag.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  3. @peff @gitster

    ident: let callers omit name with fmt_indent

    peff authored gitster committed
    Most callers want to see all of "$name <$email> $date", but
    a few want only limited parts, omitting the date, or even
    the name. We already have IDENT_NO_DATE to handle the date
    part, but there's not a good option for getting just the
    email. Callers have to done one of:
    
      1. Call ident_default_email; this does not respect
         environment variables, nor does it promise to trim
         whitespace or other crud from the result.
    
      2. Call git_{committer,author}_info; this returns the name
         and email, leaving the caller to parse out the wanted
         bits.
    
    This patch adds IDENT_NO_NAME; it stops short of adding
    IDENT_NO_EMAIL, as no callers want it (nor are likely to),
    and it complicates the error handling of the function.
    
    When no name is requested, the angle brackets (<>) around
    the email address are also omitted.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  4. @peff @gitster

    ident: refactor NO_DATE flag in fmt_ident

    peff authored gitster committed
    As a short-hand, we extract this flag into the local
    variable "name_addr_only". It's more accurate to simply
    negate this and refer to it as "want_date", which will be
    less confusing when we add more NO_* flags.
    
    While we're touching this part of the code, let's move the
    call to ident_default_date() only when we are actually going
    to use it, not when we have NO_DATE set, or when we get a
    date from the environment.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  5. @peff @gitster

    ident: reword empty ident error message

    peff authored gitster committed
    There's on point in printing the name, since it is by
    definition the empty string if we have reached this code
    path. Instead, let's be more clear that we are complaining
    about the empty name, but still show the email address that
    it is attached to (since that may provide some context to
    the user).
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on May 22, 2012
  1. @peff @gitster

    fix off-by-one error in split_ident_line

    peff authored gitster committed
    Commit 4b340cf split the logic to parse an ident line out of
    pretty.c's format_person_part. But in doing so, it
    accidentally introduced an off-by-one error that caused it
    to think that single-character names were invalid.
    
    This manifested itself as the "%an" format failing to show
    anything at all for a single-character name.
    
    Reported-by: Brian Turner <bturner@atlassian.com>
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  2. @peff @gitster

    ident: trim whitespace from default name/email

    peff authored gitster committed
    Usually these values get fed to fmt_ident, which will trim
    any cruft anyway, but there are a few code paths which use
    them directly. Let's clean them up for the benefit of those
    callers. Furthermore, fmt_ident will look at the pre-trimmed
    value and decide whether to invoke ERROR_ON_NO_NAME; this
    check can be fooled by a name consisting only of spaces.
    
    Note that we only bother to clean up when we are pulling the
    information from gecos or from system files. Any other value
    comes from a config file, where we will have cleaned up
    accidental whitespace already.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  3. @peff @gitster

    ident: use a dynamic strbuf in fmt_ident

    peff authored gitster committed
    Now that we accept arbitrary-sized names and email
    addresses, the only remaining limit is in the actual
    formatting of the names into a buffer. The current limit is
    1000 characters, which is not likely to be reached, but
    using a strbuf is one less error condition we have to worry
    about.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  4. @peff @gitster

    ident: use full dns names to generate email addresses

    peff authored gitster committed
    When we construct an email address from the username and
    hostname, we generate the host part of the email with this
    procedure:
    
      1. add the result of gethostname
    
      2. if it has a dot, ok, it's fully qualified
    
      3. if not, then look up the unqualified hostname via
         gethostbyname; take the domain name of the result and
         append it to the hostname
    
    Step 3 can actually produce a bogus result, as the name
    returned by gethostbyname may not be related to the hostname
    we fed it (e.g., consider a machine "foo" with names
    "foo.one.example.com" and "bar.two.example.com"; we may have
    the latter returned and generate the bogus name
    "foo.two.example.com").
    
    This patch simply uses the full hostname returned by
    gethostbyname. In the common case that the first part is the
    same as the unqualified hostname, the behavior is identical.
    And in the case that it is not the same, we are much more
    likely to be generating a valid name.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  5. @peff @gitster

    ident: report passwd errors with a more friendly message

    peff authored gitster committed
    When getpwuid fails, we give a cute but cryptic message.
    While it makes sense if you know that getpwuid or identity
    functions are being called, this code is triggered behind
    the scenes by quite a few git commands these days (e.g.,
    receive-pack on a remote server might use it for a reflog;
    the current message is hard to distinguish from an
    authentication error).  Let's switch to something that gives
    a little more context.
    
    While we're at it, we can factor out all of the
    cut-and-pastes of the "you don't exist" message into a
    wrapper function. Rather than provide xgetpwuid, let's make
    it even more specific to just getting the passwd entry for
    the current uid. That's the only way we use getpwuid anyway,
    and it lets us make an even more specific error message.
    
    The current message also fails to mention errno. While the
    usual cause for getpwuid failing is that the user does not
    exist, mentioning errno makes it easier to diagnose these
    problems.  Note that POSIX specifies that errno remain
    untouched if the passwd entry does not exist (but will be
    set on actual errors), whereas some systems will return
    ENOENT or similar for a missing entry. We handle both cases
    in our wrapper.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  6. @peff @gitster

    drop length limitations on gecos-derived names and emails

    peff authored gitster committed
    When we pull the user's name from the GECOS field of the
    passwd file (or generate an email address based on their
    username and hostname), we put the result into a
    static buffer. While it's extremely unlikely that anybody
    ever hit these limits (after all, in such a case their
    parents must have hated them), we still had to deal with the
    error cases in our code.
    
    Converting these static buffers to strbufs lets us simplify
    the code and drop some error messages from the documentation
    that have confused some users.
    
    The conversion is mostly mechanical: replace string copies
    with strbuf equivalents, and access the strbuf.buf directly.
    There are a few exceptions:
    
      - copy_gecos and copy_email are the big winners in code
        reduction (since they no longer have to manage the
        string length manually)
    
      - git_ident_config wants to replace old versions of
        the default name (e.g., if we read the config multiple
        times), so it must reset+add to the strbuf instead of
        just adding
    
    Note that there is still one length limitation: the
    gethostname interface requires us to provide a static
    buffer, so we arbitrarily choose 1024 bytes for the
    hostname.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  7. @peff @gitster

    ident: don't write fallback username into git_default_name

    peff authored gitster committed
    The fmt_ident function gets a flag that tells us whether to
    die if the name field is blank. If it is blank and we don't
    die, then we fall back to the username from the passwd file.
    
    The current code writes the value into git_default_name.
    However, that's not necessarily correct, as the empty value
    might have come from git_default_name, or it might have been
    passed in.  This leads to two potential problems:
    
      1. If we are overriding an empty name in the passed-in
         value, then we may be overwriting a perfectly good name
         (from gitconfig or gecos) in the git_default_name
         buffer. Later calls to fmt_ident will end up using the
         fallback name, even though a better name was available.
    
      2. If we override an empty gecos name, we end up with the
         fallback name in git_default_name. A later call that
         uses IDENT_ERROR_ON_NO_NAME will see the fallback name
         and think that it is a good name, instead of producing
         an error. In other words, a blank gecos name would
         cause an error with this code:
    
           git_committer_info(IDENT_ERROR_ON_NO_NAME);
    
         but not this:
    
           git_committer_info(0);
           git_committer_info(IDENT_ERROR_ON_NO_NAME);
    
         because in the latter case, the first call has polluted
         the name buffer.
    
    Instead, let's make the fallback a per-invocation variable.
    We can just use the pw->pw_name string directly, since it
    only needs to persist through the rest of the function (and
    we don't do any other getpwent calls).
    
    Note that while this solves (1) for future invocations of
    fmt_indent, the current invocation might use the fallback
    when it could in theory load a better value from
    git_default_name. However, by not passing
    IDENT_ERROR_ON_NO_NAME, the caller is indicating that it
    does not care too much about the name, anyway, so we don't
    bother; this is primarily about protecting future callers
    who do care.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  8. @peff @gitster

    fmt_ident: drop IDENT_WARN_ON_NO_NAME code

    peff authored gitster committed
    There are no more callers who want this, so we can drop it.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  9. @peff @gitster

    ident: trim trailing newline from /etc/mailname

    peff authored gitster committed
    We use fgets to read the /etc/mailname file, which means we
    will typically end up with an extra newline in our
    git_default_email. Most of the time this doesn't matter, as
    fmt_ident will skip it as cruft, but there is one code path
    that accesses it directly (in http-push.c:lock_remote).
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  10. @peff @gitster

    move git_default_* variables to ident.c

    peff authored gitster committed
    There's no reason anybody outside of ident.c should access
    these directly (they should use the new accessors which make
    sure the variables are initialized), so we can make them
    file-scope statics.
    
    While we're at it, move user_ident_explicitly_given into
    ident.c; while still globally visible, it makes more sense
    to reside with the ident code.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  11. @peff @gitster

    move identity config parsing to ident.c

    peff authored gitster committed
    There's no reason for this to be in config, except that once
    upon a time all of the config parsing was there. It makes
    more sense to keep the ident code together.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  12. @peff @gitster

    ident: split setup_ident into separate functions

    peff authored gitster committed
    This function sets up the default name, email, and date, and
    is not publicly available. Let's split it into three public
    functions so that callers can get just the parts they need.
    
    While we're at it, let's change the interface to simple
    accessors. The original function was called only by fmt_ident,
    and contained logic for "if we already have some other
    value, don't load the default" which properly belongs in
    fmt_ident.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Dec 20, 2010
  1. @peff @gitster

    ident: die on bogus date format

    peff authored gitster committed
    If the user gives "git commit --date=foobar", we silently
    ignore the --date flag. We should note the error.
    
    This patch puts the fix at the lowest level of fmt_ident,
    which means it also handles GIT_AUTHOR_DATE=foobar, as well.
    
    There are two down-sides to this approach:
    
      1. Technically this breaks somebody doing something like
         "git commit --date=now", which happened to work because
         bogus data is the same as "now". Though we do
         explicitly handle the empty string, so anybody passing
         an empty variable through the environment will still
         work.
    
         If the error is too much, perhaps it can be downgraded
         to a warning?
    
      2. The error checking happens _after_ the commit message
         is written, which can be annoying to the user. We can
         put explicit checks closer to the beginning of
         git-commit, but that feels a little hack-ish; suddenly
         git-commit has to care about how fmt_ident works. Maybe
         we could simply call fmt_ident earlier?
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Something went wrong with that request. Please try again.