#4549 lib/git.py: Recognize git submodules when obtaining patch stats
Opened 4 years ago by msrb. Modified 2 years ago

file modified
+3 -2
@@ -2744,8 +2744,9 @@ 

      elif hasattr(patch, "delta"):

          status = None

          # Newer pygit2

-         # we recognize non-executable file, executable file and symlink

-         expected_modes = [33188, 33261, 40960]

+         # we recognize non-executable file, executable file,

+         # symlink and a special "git submodule" directory

+         expected_modes = [33188, 33261, 40960, 57344]

          if (

              patch.delta.new_file.mode == 0

              and patch.delta.old_file.mode in expected_modes

file modified
+2 -2
@@ -17,8 +17,8 @@ 

  {% endblock %}

  {% block repo %}

  {% set splitted_message = commit.message.split('\n') %}

- {# we recognize non-executable file, executable file and symlink #}

- {% set expected_modes = [33188, 33261, 40960] %}

+ {# we recognize non-executable file, executable file, symlink and a special "git submodule" directory #}

+ {% set expected_modes = [33188, 33261, 40960, 57344] %}

  <div class="row">

    <div class="col">

        {% block overviewtabs %}{{ super() }}{% endblock %}

Fixes:
UnboundLocalError: local variable 'status' referenced before assignment

Our OSCI Pagure instance was returning 500 when there was a pull request which was adding a new git submodule into the repository. This patch fixed the problem.

Note I'd love to add some tests, but I am gonna need some guidance on where exactly those tests should go.

Thanks!

07:24:57  Failed tests:
07:24:57  FAILED test: py3-test_style

@msrb Could you please run black on your code to fix the style?

rebased onto f89713e5a0ffce715381c01cc3cdb8abe8c2a5a5

4 years ago

So the problem was:

$ python -m flake8 --ignore=E712,W503,E203 pagure/
pagure/lib/git.py:2747:80: E501 line too long (108 > 79 characters)

It should be fixed now. I force-pushed the fix, hope it's ok.

That's fine, we'll see if the tests agree. :wink:

They don't :) Let me try to figure out what is going on there...

Now the problem is not on your side, we have a broken pipeline due to some f30/pip updates

@msrb can you rebase the pr? the test problem should be fixed now

rebased onto 547c67a

4 years ago

@jlanda rebased :wink: let's see now...

We are green, folks! \o/

For the tests, I'd say feel free to add them to a new file :)

@msrb Do you need some help to get started on the tests?

Sorry for the delays guys - I will try to finalize this PR by the end of this week.

@msrb Any effort going into this now?

I wanna land this on next release. Can I finish the tests @msrb ?

Metadata Update from @jlanda:
- Request assigned

4 years ago

I'll took this silence as a yes :)

@jlanda Any chance you're planning on taking a crack at this soon?

pretty please pagure-ci rebuild

2 years ago

pretty please pagure-ci rebuild

2 years ago