1. 16 Apr, 2019 1 commit
    • Duy Nguyen's avatar
      submodule-config.c: use repo_get_oid for reading .gitmodules · d9b8b8f8
      Duy Nguyen authored
      Since 76e9bdc4 (submodule: support reading .gitmodules when it's not
      in the working tree - 2018-10-25), every time you do
          git grep --recurse-submodules
      you are likely to see one warning line per submodule (unless all those
      submodules also have submodules). On a superproject with plenty of
      submodules (I've seen one with 67) this is really annoying.
      The warning was there because we could not resolve extended SHA-1
      syntax on a submodule. We can now. Make use of the new API and get rid
      of the warning.
      It would be even better if config_with_options() supports multiple
      repositories too. But one step at a time.
      Signed-off-by: Duy Nguyen's avatarNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  2. 20 Feb, 2019 1 commit
  3. 31 Oct, 2018 1 commit
    • Antonio Ospite's avatar
      submodule: support reading .gitmodules when it's not in the working tree · 76e9bdc4
      Antonio Ospite authored
      When the .gitmodules file is not available in the working tree, try
      using the content from the index and from the current branch. This
      covers the case when the file is part of the repository but for some
      reason it is not checked out, for example because of a sparse checkout.
      This makes it possible to use at least the 'git submodule' commands
      which *read* the gitmodules configuration file without fully populating
      the working tree.
      Writing to .gitmodules will still require that the file is checked out,
      so check for that before calling config_set_in_gitmodules_file_gently.
      Add a similar check also in git-submodule.sh::cmd_add() to anticipate
      the eventual failure of the "git submodule add" command when .gitmodules
      is not safely writeable; this prevents the command from leaving the
      repository in a spurious state (e.g. the submodule repository was cloned
      but .gitmodules was not updated because
      config_set_in_gitmodules_file_gently failed).
      Moreover, since config_from_gitmodules() now accesses the global object
      store, it is necessary to protect all code paths which call the function
      against concurrent access to the global object store. Currently this
      only happens in builtin/grep.c::grep_submodules(), so call
      grep_read_lock() before invoking code involving
      Finally, add t7418-submodule-sparse-gitmodules.sh to verify that reading
      from .gitmodules succeeds and that writing to it fails when the file is
      not checked out.
      NOTE: there is one rare case where this new feature does not work
      properly yet: nested submodules without .gitmodules in their working
      tree.  This has been documented with a warning and a test_expect_failure
      item in t7814, and in this case the current behavior is not altered: no
      config is read.
      Signed-off-by: Antonio Ospite's avatarAntonio Ospite <ao2@ao2.it>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  4. 09 Oct, 2018 2 commits
  5. 27 Sep, 2018 2 commits
    • Jeff King's avatar
      submodule-config: ban submodule paths that start with a dash · 273c6149
      Jeff King authored
      We recently banned submodule urls that look like
      command-line options. This is the matching change to ban
      leading-dash paths.
      As with the urls, this should not break any use cases that
      currently work. Even with our "--" separator passed to
      git-clone, git-submodule.sh gets confused. Without the code
      portion of this patch, the clone of "-sub" added in t7417
      would yield results like:
          /path/to/git-submodule: 410: cd: Illegal option -s
          /path/to/git-submodule: 417: cd: Illegal option -s
          /path/to/git-submodule: 410: cd: Illegal option -s
          /path/to/git-submodule: 417: cd: Illegal option -s
          Fetched in submodule path '-sub', but it did not contain b56243f8f4eb91b2f1f8109452e659f14dd3fbe4. Direct fetching of that commit failed.
      Moreover, naively adding such a submodule doesn't work:
        $ git submodule add $url -sub
        The following path is ignored by one of your .gitignore files:
      even though there is no such ignore pattern (the test script
      hacks around this with a well-placed "git mv").
      Unlike leading-dash urls, though, it's possible that such a
      path _could_ be useful if we eventually made it work. So
      this commit should be seen not as recommending a particular
      policy, but rather temporarily closing off a broken and
      possibly dangerous code-path. We may revisit this decision
      There are two minor differences to the tests in t7416 (that
      covered urls):
        1. We don't have a "./-sub" escape hatch to make this
           work, since the submodule code expects to be able to
           match canonical index names to the path field (so you
           are free to add submodule config with that path, but we
           would never actually use it, since an index entry would
           never start with "./").
        2. After this patch, cloning actually succeeds. Since we
           ignore the submodule.*.path value, we fail to find a
           config stanza for our submodule at all, and simply
           treat it as inactive. We still check for the "ignoring"
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
    • Jeff King's avatar
      submodule-config: ban submodule urls that start with dash · f6adec4e
      Jeff King authored
      The previous commit taught the submodule code to invoke our
      "git clone $url $path" with a "--" separator so that we
      aren't confused by urls or paths that start with dashes.
      However, that's just one code path. It's not clear if there
      are others, and it would be an easy mistake to add one in
      the future. Moreover, even with the fix in the previous
      commit, it's quite hard to actually do anything useful with
      such an entry. Any url starting with a dash must fall into
      one of three categories:
       - it's meant as a file url, like "-path". But then any
         clone is not going to have the matching path, since it's
         by definition relative inside the newly created clone. If
         you spell it as "./-path", the submodule code sees the
         "/" and translates this to an absolute path, so it at
         least works (assuming the receiver has the same
         filesystem layout as you). But that trick does not apply
         for a bare "-path".
       - it's meant as an ssh url, like "-host:path". But this
         already doesn't work, as we explicitly disallow ssh
         hostnames that begin with a dash (to avoid option
         injection against ssh).
       - it's a remote-helper scheme, like "-scheme::data". This
         _could_ work if the receiver bends over backwards and
         creates a funny-named helper like "git-remote--scheme".
         But normally there would not be any helper that matches.
      Since such a url does not work today and is not likely to do
      anything useful in the future, let's simply disallow them
      entirely. That protects the existing "git clone" path (in a
      belt-and-suspenders way), along with any others that might
      Our tests cover two cases:
        1. A file url with "./" continues to work, showing that
           there's an escape hatch for people with truly silly
           repo names.
        2. A url starting with "-" is rejected.
      Note that we expect case (2) to fail, but it would have done
      so even without this commit, for the reasons given above.
      So instead of just expecting failure, let's also check for
      the magic word "ignoring" on stderr. That lets us know that
      we failed for the right reason.
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  6. 29 Aug, 2018 1 commit
    • Jeff King's avatar
      convert "oidcmp() != 0" to "!oideq()" · 9001dc2a
      Jeff King authored
      This is the flip side of the previous two patches: checking
      for a non-zero oidcmp() can be more strictly expressed as
      inequality. Like those patches, we write "!= 0" in the
      coccinelle transformation, which covers by isomorphism the
      more common:
        if (oidcmp(E1, E2))
      As with the previous two patches, this patch can be achieved
      almost entirely by running "make coccicheck"; the only
      differences are manual line-wrap fixes to match the original
      There is one thing to note for anybody replicating this,
      though: coccinelle 1.0.4 seems to miss the case in
      builtin/tag.c, even though it's basically the same as all
      the others. Running with 1.0.7 does catch this, so
      presumably it's just a coccinelle bug that was fixed in the
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  7. 03 Jul, 2018 1 commit
  8. 26 Jun, 2018 6 commits
  9. 22 May, 2018 1 commit
    • Jeff King's avatar
      submodule-config: verify submodule names as paths · 0383bbb9
      Jeff King authored
      Submodule "names" come from the untrusted .gitmodules file,
      but we blindly append them to $GIT_DIR/modules to create our
      on-disk repo paths. This means you can do bad things by
      putting "../" into the name (among other things).
      Let's sanity-check these names to avoid building a path that
      can be exploited. There are two main decisions:
        1. What should the allowed syntax be?
           It's tempting to reuse verify_path(), since submodule
           names typically come from in-repo paths. But there are
           two reasons not to:
             a. It's technically more strict than what we need, as
                we really care only about breaking out of the
                $GIT_DIR/modules/ hierarchy.  E.g., having a
                submodule named "foo/.git" isn't actually
                dangerous, and it's possible that somebody has
                manually given such a funny name.
             b. Since we'll eventually use this checking logic in
                fsck to prevent downstream repositories, it should
                be consistent across platforms. Because
                verify_path() relies on is_dir_sep(), it wouldn't
                block "foo\..\bar" on a non-Windows machine.
        2. Where should we enforce it? These days most of the
           .gitmodules reads go through submodule-config.c, so
           I've put it there in the reading step. That should
           cover all of the C code.
           We also construct the name for "git submodule add"
           inside the git-submodule.sh script. This is probably
           not a big deal for security since the name is coming
           from the user anyway, but it would be polite to remind
           them if the name they pick is invalid (and we need to
           expose the name-checker to the shell anyway for our
           test scripts).
           This patch issues a warning when reading .gitmodules
           and just ignores the related config entry completely.
           This will generally end up producing a sensible error,
           as it works the same as a .gitmodules file which is
           missing a submodule entry (so "submodule update" will
           barf, but "git clone --recurse-submodules" will print
           an error but not abort the clone.
           There is one minor oddity, which is that we print the
           warning once per malformed config key (since that's how
           the config subsystem gives us the entries). So in the
           new test, for example, the user would see three
           warnings. That's OK, since the intent is that this case
           should never come up outside of malicious repositories
           (and then it might even benefit the user to see the
           message multiple times).
      Credit for finding this vulnerability and the proof of
      concept from which the test script was adapted goes to
      Etienne Stalmans.
      Signed-off-by: 's avatarJeff King <peff@peff.net>
  10. 16 May, 2018 1 commit
    • Stefan Beller's avatar
      object-store: move object access functions to object-store.h · cbd53a21
      Stefan Beller authored
      This should make these functions easier to find and cache.h less
      overwhelming to read.
      In particular, this moves:
      - read_object_file
      - oid_object_info
      - write_object_file
      As a result, most of the codebase needs to #include object-store.h.
      In this patch the #include is only added to files that would fail to
      compile otherwise.  It would be better to #include wherever
      identifiers from the header are used.  That can happen later
      when we have better tooling for it.
      Signed-off-by: Stefan Beller's avatarStefan Beller <sbeller@google.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  11. 02 May, 2018 1 commit
  12. 29 Mar, 2018 3 commits
  13. 14 Mar, 2018 1 commit
    • brian m. carlson's avatar
      sha1_file: convert read_sha1_file to struct object_id · b4f5aca4
      brian m. carlson authored
      Convert read_sha1_file to take a pointer to struct object_id and rename
      it read_object_file.  Do the same for read_sha1_file_extended.
      Convert one use in grep.c to use the new function without any other code
      change, since the pointer being passed is a void pointer that is already
      initialized with a pointer to struct object_id.  Update the declaration
      and definitions of the modified functions, and apply the following
      semantic patch to convert the remaining callers:
      expression E1, E2, E3;
      - read_sha1_file(E1.hash, E2, E3)
      + read_object_file(&E1, E2, E3)
      expression E1, E2, E3;
      - read_sha1_file(E1->hash, E2, E3)
      + read_object_file(E1, E2, E3)
      expression E1, E2, E3, E4;
      - read_sha1_file_extended(E1.hash, E2, E3, E4)
      + read_object_file_extended(&E1, E2, E3, E4)
      expression E1, E2, E3, E4;
      - read_sha1_file_extended(E1->hash, E2, E3, E4)
      + read_object_file_extended(E1, E2, E3, E4)
      Signed-off-by: brian m. carlson's avatarbrian m. carlson <sandals@crustytoothpaste.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  14. 14 Feb, 2018 1 commit
  15. 07 Aug, 2017 1 commit
  16. 03 Aug, 2017 2 commits
    • Brandon Williams's avatar
      submodule-config: lazy-load a repository's .gitmodules file · ff6f1f56
      Brandon Williams authored
      In order to use the submodule-config subsystem, callers first need to
      initialize it by calling 'repo_read_gitmodules()' or
      'gitmodules_config()' (which just redirects to
      'repo_read_gitmodules()').  There are a couple of callers who need to
      load an explicit revision of the repository's .gitmodules file (grep) or
      need to modify the .gitmodules file so they would need to load it before
      modify the file (checkout), but the majority of callers are simply
      reading the .gitmodules file present in the working tree.  For the
      common case it would be nice to avoid the boilerplate of initializing
      the submodule-config system before using it, so instead let's perform
      lazy-loading of the submodule-config system.
      Remove the calls to reading the gitmodules file from ls-files to show
      that lazy-loading the .gitmodules file works.
      Signed-off-by: 's avatarBrandon Williams <bmwill@google.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
    • Brandon Williams's avatar
      submodule-config: move submodule-config functions to submodule-config.c · 1b796ace
      Brandon Williams authored
      Migrate the functions used to initialize the submodule-config to
      submodule-config.c so that the callback routine used in the
      initialization process can be static and prevent it from being used
      outside of initializing the submodule-config through the main API.
      Signed-off-by: 's avatarBrandon Williams <bmwill@google.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  17. 02 Aug, 2017 1 commit
    • Brandon Williams's avatar
      submodule: remove submodule.fetchjobs from submodule-config parsing · f20e7c1e
      Brandon Williams authored
      The '.gitmodules' file should only contain information pertinent to
      configuring individual submodules (name to path mapping, URL where to
      obtain the submodule, etc.) while other configuration like the number of
      jobs to use when fetching submodules should be a part of the
      repository's config.
      Remove the 'submodule.fetchjobs' configuration option from the general
      submodule-config parsing and instead rely on using the
      'config_from_gitmodules()' in order to maintain backwards compatibility
      with this config being placed in the '.gitmodules' file.
      Signed-off-by: 's avatarBrandon Williams <bmwill@google.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  18. 17 Jul, 2017 1 commit
  19. 05 Jul, 2017 1 commit
  20. 30 Jun, 2017 1 commit
    • Stefan Beller's avatar
      hashmap.h: compare function has access to a data field · 7663cdc8
      Stefan Beller authored
      When using the hashmap a common need is to have access to caller provided
      data in the compare function. A couple of times we abuse the keydata field
      to pass in the data needed. This happens for example in patch-ids.c.
      This patch changes the function signature of the compare function
      to have one more void pointer available. The pointer given for each
      invocation of the compare function must be defined in the init function
      of the hashmap and is just passed through.
      Documentation of this new feature is deferred to a later patch.
      This is a rather mechanical conversion, just adding the new pass-through
      parameter.  However while at it improve the naming of the fields of all
      compare functions used by hashmaps by ensuring unused parameters are
      prefixed with 'unused_' and naming the parameters what they are (instead
      of 'unused' make it 'unused_keydata').
      Signed-off-by: Stefan Beller's avatarStefan Beller <sbeller@google.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  21. 24 Jun, 2017 1 commit
  22. 23 Jun, 2017 1 commit
  23. 15 Jun, 2017 1 commit
  24. 16 Mar, 2017 1 commit
    • Stefan Beller's avatar
      update submodules: add submodule config parsing · d601fd09
      Stefan Beller authored
      Similar to b33a15b0 (push: add recurseSubmodules config option,
      2015-11-17) and 027771fc (submodule: allow erroneous values for the
      fetchRecurseSubmodules option, 2015-08-17), we add submodule-config code
      that is later used to parse whether we are interested in updating
      We need the `die_on_error` parameter to be able to call this parsing
      function for the config file as well, which if incorrect lets Git die.
      As we're just touching the header file, also mark all functions extern.
      Signed-off-by: Stefan Beller's avatarStefan Beller <sbeller@google.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  25. 15 Mar, 2017 1 commit
  26. 22 Dec, 2016 1 commit
  27. 20 Dec, 2016 1 commit
  28. 22 Nov, 2016 2 commits
  29. 01 Aug, 2016 1 commit