Skip to content

Conversation

@eli-schwartz
Copy link
Member

@eli-schwartz eli-schwartz commented Sep 2, 2021

Fixes various inconsistencies:

  • gitattributes is respected
    • export-subst
    • export-ignore
  • submodules with relative paths are not checked out relative to the
    local clone (which does not work anyway)
  • no need to manually remove gitfiles with inaccurate heuristics

Fixes #2287
Fixes #3081
Fixes #8144

@codecov
Copy link

codecov bot commented Sep 2, 2021

Codecov Report

Merging #9210 (59a3d48) into master (1dbb6d6) will decrease coverage by 0.00%.
The diff coverage is 59.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9210      +/-   ##
==========================================
- Coverage   66.82%   66.82%   -0.01%     
==========================================
  Files         386      386              
  Lines       84834    84844      +10     
  Branches    17509    17507       -2     
==========================================
+ Hits        56690    56696       +6     
- Misses      23361    23365       +4     
  Partials     4783     4783              
Impacted Files Coverage Δ
mesonbuild/mdist.py 74.79% <59.37%> (+0.52%) ⬆️
mesonbuild/scripts/vcstagger.py 87.50% <0.00%> (-4.17%) ⬇️
mesonbuild/mesonlib/universal.py 81.53% <0.00%> (-0.25%) ⬇️
mdist.py 74.79% <0.00%> (+0.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1dbb6d6...59a3d48. Read the comment docs.

@xclaesse
Copy link
Member

xclaesse commented Sep 2, 2021

I've always wondered why we were not doing that and have been assuming @jpakkane had a reason.

@eli-schwartz
Copy link
Member Author

The lint_mypy failure is not actually my fault. @mensinda is there some way to automatically tell that open() doesn't take an encoding parameter because it's from the tarfile module, not the built-in function or io.open?

@eli-schwartz
Copy link
Member Author

@xclaesse the reason is "because submodules looked hard". :)

@mensinda
Copy link
Member

mensinda commented Sep 2, 2021 via email

@eli-schwartz
Copy link
Member Author

I was hoping for something slightly more "automatic", but I guess the manually manual way works too...

@xclaesse
Copy link
Member

xclaesse commented Sep 2, 2021

@eli-schwartz fair enough. Note that we now also support dist for a subproject that is in the same git repo as its parent project. We're going to use that feature for GStreamer. IIRC I made a unit test for that, so CI should tell if that still works.

@eli-schwartz
Copy link
Member Author

eli-schwartz commented Sep 2, 2021

Maybe that's the reason why CI fails in the "samerepo" setup...

EDIT: yes I think it is. I changed the signature of copy_git no less than 4 times before committing, and I started off with args, not kwargs, so now for the subproject case only, I'm passing subdir= as revision=

@xclaesse
Copy link
Member

xclaesse commented Sep 2, 2021

The lint_mypy failure is not actually my fault. @mensinda is there some way to automatically tell that open() doesn't take an encoding parameter because it's from the tarfile module, not the built-in function or io.open?

Probably that syntax: tar = tarfile.open(xz_distfile, "r:xz") # [ignore encoding]

@xclaesse
Copy link
Member

xclaesse commented Sep 2, 2021

Maybe that's the reason why CI fails in the "samerepo" setup...

It's failing for the --include-subprojects test case, that's another pitfall that I forgot.

@xclaesse
Copy link
Member

xclaesse commented Sep 2, 2021

@eli-schwartz wow nice, didn't know git archive has a subdir parameter, that's exactly what we need for the release a subproject case. I think you got that part right already.

However the --include-subprojects case is more tricky because it means adding files that are not tracker by git. I guess it means you need to do a local clone, copy subprojects excluding .git files, "git add" them, do the archive ?

@xclaesse
Copy link
Member

xclaesse commented Sep 2, 2021

subprojects could themself have submodule... recursively... not sure that case ever worked tbh.

Fixes various inconsistencies:
- gitattributes is respected
  - export-subst
  - export-ignore
- submodules with relative paths are not checked out relative to the
  local clone (which does not work anyway)
- no need to manually remove gitfiles with inaccurate heuristics

Fixes mesonbuild#2287
Fixes mesonbuild#3081
Fixes mesonbuild#8144
@eli-schwartz
Copy link
Member Author

  • free function rename now that it doesn't actually clone anything
  • the case of project root not being repo root, is now corrected to actually pass in the temporary distdir for copying (oops) and pass the subdir as subdir= instead of revision= (also oops)

And with that, ./run_unittests.py test_dist_git as flagged by the CI as a relevant test to verify this PR... now passes locally.

I have a local configuration

tag.forcesignannotated=true
commit.gpgsign=true

This causes the tests to fail with e.g.

error: gpg failed to sign the data
fatal: failed to write commit object

Since this is a unittest, it is never wrong to tell git "just ignore
prior configuration, and disable all PGP signing".
@eli-schwartz
Copy link
Member Author

(I lied, I have local gitconfig which borks things up. Might as well add the changes I needed here too.)

@xclaesse
Copy link
Member

xclaesse commented Sep 3, 2021

LGTM, seems to be a safer way indeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dist failure with relative submodule paths Exclude files from ninja dist? ninja dist should respect gitattributes export-ignore

3 participants