Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wrap: Fix diff_files not finding the patch to apply #10602

Merged

Conversation

nirbheek
Copy link
Member

@nirbheek nirbheek commented Jul 14, 2022

The path to the patch must be provided relative to the working directory used to run the command: patch or git.

The feature doesn't work when using meson subprojects update without this fix.

@nirbheek nirbheek added this to the 0.63.1 milestone Jul 14, 2022
@nirbheek nirbheek requested a review from jpakkane as a code owner July 14, 2022 22:50
@xclaesse
Copy link
Member

Don't we have a unit test for this feature?

@codecov
Copy link

codecov bot commented Jul 14, 2022

Codecov Report

Merging #10602 (f46c9a3) into master (f6a54de) will decrease coverage by 18.00%.
The diff coverage is n/a.

❗ Current head f46c9a3 differs from pull request most recent head 3e65456. Consider uploading reports for the commit 3e65456 to get more accurate results

@@             Coverage Diff             @@
##           master   #10602       +/-   ##
===========================================
- Coverage   68.31%   50.31%   -18.01%     
===========================================
  Files         203      203               
  Lines       44000    44000               
  Branches     9773     9755       -18     
===========================================
- Hits        30060    22139     -7921     
- Misses      11619    19859     +8240     
+ Partials     2321     2002      -319     
Impacted Files Coverage Δ
modules/qt5.py 0.00% <0.00%> (-100.00%) ⬇️
modules/modtest.py 0.00% <0.00%> (-100.00%) ⬇️
scripts/run_tool.py 0.00% <0.00%> (-100.00%) ⬇️
scripts/msgfmthelper.py 0.00% <0.00%> (-100.00%) ⬇️
modules/keyval.py 0.00% <0.00%> (-95.24%) ⬇️
scripts/clangtidy.py 0.00% <0.00%> (-93.34%) ⬇️
scripts/vcstagger.py 0.00% <0.00%> (-91.67%) ⬇️
modules/unstable_icestorm.py 0.00% <0.00%> (-91.67%) ⬇️
modules/sourceset.py 0.00% <0.00%> (-90.54%) ⬇️
scripts/depscan.py 0.00% <0.00%> (-83.92%) ⬇️
... and 117 more

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 5851c13...3e65456. Read the comment docs.

@nirbheek
Copy link
Member Author

Don't we have a unit test for this feature?

It works in the test because wrap.filesdir is an absolute path, probably because interpreter.py passes it an absolute path to the source directory. If you use meson subprojects update, the source directory is relative, and things fail.

Fixing merge request description...

@eli-schwartz
Copy link
Member

Can we fix the commit description too? :)

When `self.wrap.filesdir` is a relative path, which happens when
`meson subprojects update` is run, the path to the patch must be
provided relative to the working directory in which `patch` or `git`
is run.

`self.wrap.filesdir` is absolute when `Resolve()` is invoked by the
Meson interpreter, which is why this wasn't detected by the tests.
@nirbheek nirbheek force-pushed the wrap-diff-files-cant-find-patch branch from 5db9c07 to 3e65456 Compare July 15, 2022 00:42
@nirbheek
Copy link
Member Author

Done.

@jpakkane jpakkane merged commit c19773c into mesonbuild:master Jul 15, 2022
@nirbheek nirbheek deleted the wrap-diff-files-cant-find-patch branch July 16, 2022 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants