Read about our upcoming Code of Conduct on this issue

  1. 25 Nov, 2020 1 commit
  2. 16 Nov, 2020 1 commit
    • Georges Racinet's avatar
      Branch acces checks: direct identification of legit merges · 948dac54b949
      Georges Racinet authored
      Closes #368
      
      To let merges done from the Rails application be checked
      against the distinct set of merge rules rather that push
      rules, GitLab looks for MRs matching the pre-receive changes by
      comparing with the merge commit sha that's always available
      (and set by the Repository Model) before the actual branch move.
      
      Because the Mercurial change can be atomic, we have no other
      option than passing a special parameter down from
      `Repository#merge` through the Mercurial backend, with the
      `HEPTAPOD_ACCEPT_MERGE_ID` environment variable.
      In turn the Mercurial process will add it to its pre-receive
      hook payload, and the target branch can be compared to switch
      from (direct) push to (Rails) merge mode.
      
      In the case of merges intended from the very beginning to be
      fast forwards, the commit sha is already known in advance,
      but we're using the same mechanism for consistency in the
      check.
      
      Finally, there was a good dose of wrestling with those RSpec
      examples that care about merging to a protected branch, because
      of the on-demand nature of instantiation.
      
      --HG--
      branch : heptapod-stable
      948dac54b949
  3. 15 Nov, 2020 1 commit
  4. 07 Nov, 2020 1 commit
    • Georges Racinet's avatar
      multi-vcs: validate Project vcs_type against Application Settings · e90fbe3ee00f
      Georges Racinet authored
      Now that we have the `vcs_types` Application Settings field,
      we use it to enforce upon Project creation that the selected
      `vcs_type` is among them.
      
      This validation has to be done by a separate method in order
      to access the `CurrentSettings` object.
      
      Also, we've considered making the default value for `vcs_type` in
      `CreateService` not to be the library-level default, so that
      explicitely passing a value is never required, even in the case where
      the library default is not allowed by Application Settings, but
      finally decided that failing with an explicit message in that case
      was better than such magic. Note also that the Web UI will always
      pass a value, so this case is in practice for API calls only.
      
      --HG--
      branch : heptapod
      e90fbe3ee00f
  5. 05 Nov, 2020 1 commit
  6. 31 Oct, 2020 1 commit
    • Georges Racinet's avatar
      Project model: validation of vcs_type · 53af134a42a1
      Georges Racinet authored
      This is simply ensuring the value is a support type
      
      We're testing this twice (model and service) because `project_spec`
      is not yet in the Heptapod CI shortlist and it takes too much time
      that such a trivial test would be worth it. Of course, it has
      its intrinsic value, and we will activate it soonish, but that's not
      the current concern. Also this prepares for potentially more advanced
      behaviour at the service level.
      
      --HG--
      branch : heptapod
      53af134a42a1
  7. 03 Nov, 2020 1 commit
  8. 28 Oct, 2020 3 commits
    • Georges Racinet on mutations.racinet.fr's avatar
      Commit model: using hpd_short_display_id in Note links text · dfc309bc925d
      Now that we have this agnostic method, we can use it directly
      rather than duplicating its logic (even if simple).
      
      Also, this ones the last use of `short_hgid`, meaning we
      should now be able to drop it, unless other cleanups lead us
      to prefer it to the currently method in use in some place
      (MRs and pipelines, notably).
      
      --HG--
      branch : heptapod
      dfc309bc925d
    • Georges Racinet on mutations.racinet.fr's avatar
      Commit model: hpd_short_display_id · 9c9230fcb76f
      Whereas `#hpd_display_id` is implemented on the raw commit, this
      one is easier at the model level, the intended result depending only
      on whether it's Mercurial or not.
      
      It seems like a good idea to let only the "app" layer of the Rails
      application bother about short ids, which are after all presentation
      businesss - unless we later find out the dance with the `truncate_sha`
      classmethod is a performance sink.
      
      Test is minimal, but enough to guarantee we're calling other methods
      with the proper arguments.
      
      --HG--
      branch : heptapod
      9c9230fcb76f
    • Georges Racinet on mutations.racinet.fr's avatar
      Commit model: making truncate_sha optionally VCS-aware · 2cd1251472b2
      This being a class method, we have to pass a `context` explicitely,
      but it'll give us a chance to keep duplication low on generation of
      short SHAs.
      
      --HG--
      branch : heptapod
      2cd1251472b2
  9. 26 Oct, 2020 1 commit
    • Georges Racinet's avatar
      User#can_create_project_in_group? takes Group config into account · 15937daf2934
      Georges Racinet authored
      This concludes #361: the previous implementation was only
      checking if there's at least a group where the user has Developer
      role, whereas it actually depends on the group configuration.
      
      It's possible that this is partly redundant with
      `groups_with_developer_maintainer_project_access`, but it's
      hard to be sure, and we're careful to stop as soon as we have
      at least a match.
      
      --HG--
      branch : heptapod-stable
      15937daf2934
  10. 19 Sep, 2020 1 commit
  11. 10 Sep, 2020 2 commits
    • Georges Racinet's avatar
      Markdown renderings: lookup for Mercurial short nodes · 31146d7c450d
      Georges Racinet authored
      The `CommitReferenceFilter` class is the one responsible for
      the rendering.
      
      We're *adding* resolution of truncated Mercurial Node ids for
      the case of the `hg_git` VCS type, relying on the existing
      map of full Mercurial SHA (Node id) to Git SHA.
      
      This is very inefficient, and proper calls with low startup
      overhead to a persistent Mercurial nodemap would do much better
      above a certain repository size (persistent nodemap is provided
      by Rust native extensions, and low overhead would be achieved only
      by a fastpath without Python or a long-running process, such as
      HGitaly)
      
      While the `Git::Commit.batch_by_oid` method
      doesn't seem to be called except from the `#commits_by` method of the
      `Repository` model, the latter is used in several places
      of the application, with risks of breakage and/or severe performance
      degradation. For `hg_git` VCS type, most callers don't need the Mercurial
      resolution. That's why we're executing the new lookup only if
      the new `hg_full_resolution` argument is set to `true`.
      
      Preliminary performance analysis (non scientific, on my workstation)
      shows that in the 100k changesets range (pypy repo) this naive lookup takes
      about 30ms, whereas `hg log -T` is in the 100-200ms ballpark.
      Around 500k changesets (mozilla-central repo), times have the same order
      of magnitude (around 100-200ms again). To insist, this is with the hg startup
      overhead and without the Rust persistent nodemap (would take less than 1ms).
      We fully expect the current hg<->git maps to be an unbereable performance
      problem in Heptapod anyway around 500k to 1M changesets anyway, only solved
      by HGitaly.
      
      All in all, the performance question seems to be acceptable in the current
      inefficient context *for Note rendering*.
      HGitaly would provide an efficient RPC call anyway.
      
      --HG--
      branch : heptapod-stable
      31146d7c450d
    • Georges Racinet's avatar
      Markdown rendering for links to Mercurial changesets · 42a5f760c1a1
      Georges Racinet authored
      This fixes the rendering of all links to Mercurial changesets in
      Markdown (#342), because the changed method is the one called from
      `lib/banzai/filter/abstract_reference_filter`.
      
      In particular, the "system note" displaying additional commits in MRs
      are actually rendered server side from Markdown produced by
      `SystemNotes::CommitService#commits_list`.
      
      This doesn't change the lookup, i.e., the SHA prefix to render is still
      the Git one.
      
      Also, the rendering of `Note` is persisted in the database, in the
      `note_html` column, so this change doesn't apply to existing Notes.
      We could have a background migration for this, or simply a mass
      invalidation if that is suitable.
      
      --HG--
      branch : heptapod
      42a5f760c1a1
  12. 09 Sep, 2020 1 commit
    • Georges Racinet's avatar
      Repository model: fixed import method if project is nil · 5ec5e5e5f109
      Georges Racinet authored
      This was spotted by the development setup seeding snippets.
      Probably the novelty in GitLab 13.3 is to go straight through
      this instead of `Gitlab::Shell`. The point is that a Repository
      is not necessarily tied to a Project any more, it has a container,
      which can itself have a Project, or not. Snippets can exist
      independently of any Project.
      
      --HG--
      branch : heptapod
      5ec5e5e5f109
  13. 04 Sep, 2020 1 commit
  14. 01 Sep, 2020 1 commit
  15. 28 Aug, 2020 1 commit
  16. 27 Aug, 2020 1 commit
    • Georges Racinet's avatar
      Forwarding actual permissions to Mercurial processes · 4c45dfe84231
      Georges Racinet authored
      Closes #334
      
      Normally, the abstraction layers in GitLab mean that the Project
      should not be accessible from the low level repository. However,
      we need it to pass it down to the `hg` processes. Doing it in
      `hg_env_for_write` is the way to make it really robust, since all
      write operations basically have to use it.
      
      To mitigate somewhat this breach of abstraction, we make the
      attribute writable only, and we give it a more specific name than
      just '@project'. Aesthetically, it would be better to store only
      a factory for the `UserAccess` instance that we really need, but
      that could be done separately.
      
      This required running some of `HgAccess` RSpec tests with empty
      repositories, because in them, the project owner does not have enough
      rights to create a first commit any more, and there's not even
      an admin user around for that.
      
      --HG--
      branch : heptapod
      4c45dfe84231
  17. 24 Aug, 2020 2 commits
    • Georges Racinet's avatar
      Repository files (multi_action): check publisher permission · e029134a44f0
      Georges Racinet authored
      This is a variant of #332 for multi_action.
      
      As with the case of merge requests, the check should also be
      performed at low level by Mercurial, but doing it at the model
      level makes for early failure, avoiding atomicity issues.
      
      A difference with MRs is that we don't have a simple way *not* to
      propose the action to the user yet, because higher layers are more
      complicated and don't involve checks, since they are meant to produce
      forks for users without write permission.
      
      This currently provides 500 errors, but at least it closes the
      security problem, and we can later on add the necessary catching.
      
      We didn't want to affect Wikis, because for them, allowing publication
      for Developers is pretty normal.
      
      --HG--
      branch : heptapod-stable
      e029134a44f0
    • Georges Racinet's avatar
      Merge Requests: only publishers are allowed to merge · 7ef09f4b1371
      Georges Racinet authored
      Part of #332
      
      This is to be understood as somewhat temporary,
      making sense only because currently merges from the Rails app always
      involve publishing, which can change for various reasons.
      
      It has the following advantages:
      
      - merge button will be disabled for ordinary Developers, hence
        avoiding obscure error message
      - in case users find their way to click the button anyway (or
        just use the API), this fails early, whereas only enforcing
        the publishing permission in Mercurial itself would only refuse
        the phase change, meaning that a draft merge commit could still have
        been created.
      
      --HG--
      branch : heptapod-stable
      7ef09f4b1371
  18. 20 Aug, 2020 1 commit
  19. 05 Aug, 2020 1 commit
  20. 29 Jul, 2020 1 commit
  21. 27 Jul, 2020 1 commit
    • Georges Racinet's avatar
      CI Pipelines: adding HG specific variables about commit · b533cac35cbf
      Georges Racinet authored
      Closes #316
      
      We should not need them once we are using the native (HGitaly) mode
      but we'll have to maintain a sensible value from then for backwards
      compatibility: at this point `*_HG_SHA` and `*_SHA` will have to
      coincide.
      
      --HG--
      branch : heptapod-stable
      b533cac35cbf
  22. 23 Jul, 2020 1 commit
  23. 20 Jul, 2020 1 commit
  24. 25 Jul, 2020 1 commit
    • Georges Racinet's avatar
      Fixed RSpec tests for MergeRequest model · a08291771946
      Georges Racinet authored
      The fix is simply avoiding conversions from Git SHA to Mercurial
      for Git repositories. All repositories created by the upstream
      GitLab factories are and will stay Git ones.
      
      --HG--
      branch : heptapod
      a08291771946
  25. 16 Jul, 2020 2 commits
    • Georges Racinet's avatar
      Repository model: using `delegate` · da933348c308
      Georges Racinet authored
      It's nicer than explicit forwarding in that it'd adapt to
      all argument passing explicitely.
      
      --HG--
      branch : heptapod-stable
      da933348c308
    • Georges Racinet's avatar
      Making versioned snippets work with Git on Heptapod · b9bfd3279c3e
      Georges Racinet authored
      Closes #294
      
      Some Snippet repositories aren't associated with any Project,
      but they still have an abstract `container`, which will itself
      be an instance of a `Snippet` model class. This only follows
      the general GitLab trend.
      
      As a consequence, the Snippet models must have the methods
      related to VCS types – and also the ones meant for Mercurial
      repository creation (for namespaces and subgroups).
      
      If later on we want to have Mercurial snippet repositories,
      assuming the security class `HgAccessSnippet` is ready, it
      would be only a matter of changing the `vcs_type` method on
      Snippet classes. Probably, that would entail adding a `vcs_type`
      column in th `personal_snippets` table.
      
      --HG--
      branch : heptapod
      b9bfd3279c3e
  26. 29 Jun, 2020 2 commits
  27. 25 Jun, 2020 1 commit
  28. 18 Jun, 2020 1 commit
  29. 14 Jun, 2020 2 commits
    • Georges Racinet's avatar
      Read the Docs: fixing and enabling connection test · 8843a066c5ae
      Georges Racinet authored
      Closes #289
      
      Turns out the test data is perfectly suitable, because it uses the
      real default branch name of the project, BUT it is a regular hash
      whose keys are symbols. Hence our `push_data["ref"]` wouldn't work
      for it
      
      On the other hand, real push data is a `HashWithIndifferentAccess`,
      meaning that we can use symbols in both cases.
      
      Found that logging the ref only after all parsing has succeeded was
      pointless, replaced by resulting Mercurial branch.
      
      --HG--
      branch : heptapod-stable
      8843a066c5ae
    • Georges Racinet's avatar
      Passing vcs_type to GitlabShellWorker and fixing repo removal · 1f9e56d13b01
      Georges Racinet authored
      Because sidekiq workers don't seem to take named arguments, we
      pass it as the last required positional one and convert it to
      named argument in the actual call to `Gitlab::Shell`.
      
      Then it was simpler just to let `Gitlab::Shell#rm_namespace`, the
      only method not taking a `vcs_type` named arg to accept it for
      uniformity.
      
      All call sites for GitlabShellWorker are updated. There aren't so
      many, due to the general deprecation of `Gitlab::Shell`, also a
      reason not to worry about new callers appearing in future versions.
      
      Finally, the `remove` method wasn't even implemented in
      `HgGitRepository`.
      
      --HG--
      branch : heptapod
      1f9e56d13b01
  30. 13 Jun, 2020 2 commits
    • Georges Racinet's avatar
      Fixed Mercurial repository HTTP and SSH URLs. · 90c369ed30f8
      Georges Racinet authored
      The generating logic was moved to the separate `HasRepository`
      concern, perhaps another change due to the introduction of
      snippet repositories.
      
      Rather than introducing Mercurial knowledge in the underlying
      lib `RepositoryUrlBuilder` module (would have to carry it over
      in all function calls),
      we preferred this time to change its semantics: the extension
      part (`.git`) is handled by the caller, the concern in our case.
      
      Perhaps in the longer run we'll have to do it in the mixin module
      anyway (another caller in app/helpers/submodule_helper.rb)
      
      --HG--
      branch : heptapod
      90c369ed30f8
    • Georges Racinet's avatar
      Shortcut for mercurial? etc on Repository model · a2a10377274b
      Georges Racinet authored
      A bit tired of these `repository.project.vcs_type` and besides,
      that's bad style.
      
      --HG--
      branch : heptapod
      a2a10377274b
  31. 11 Jun, 2020 2 commits
    • Georges Racinet's avatar
      Read the Docs: skipping verification · ab948d0e6ae5
      Georges Racinet authored
      The test and perhaps test_data methods should be implemented.
      It's too late for the imminent 0.13.1 release
      
      --HG--
      branch : heptapod-stable
      ab948d0e6ae5
    • Georges Racinet's avatar
      Read the Docs: avoiding whitespace in token · ebea12eee37d
      Georges Racinet authored
      The copy/paste from the Read the Docs integration page is hard
      to do in practice without leading or trailing whitespace.
      
      It would be better to do that stripping earlier, when the token is
      set, but that's a bit less obvious. This version will be good enough
      for the time being.
      
      --HG--
      branch : heptapod-stable
      ebea12eee37d