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

patch level guessed incorrectly? #4241

Open
ltalirz opened this issue Jun 25, 2021 · 5 comments · May be fixed by #5485
Open

patch level guessed incorrectly? #4241

ltalirz opened this issue Jun 25, 2021 · 5 comments · May be fixed by #5485
Labels
stale::recovered [bot] recovered after being marked as stale stale [bot] marked as stale due to inactivity

Comments

@ltalirz
Copy link

ltalirz commented Jun 25, 2021

Actual Behavior

When supplying a patch (produced by git format-patch) that creates a new file, it appears that conda-build (3.21.4) guesses the patch level incorrectly, resulting in the file being created in the wrong directory.

In the following example, I am applying two patches:

  • config.patch that modifies an existing file at the top-level of the package
  • cmake.patch that creates a new file at the top-level

As shown in the build log, the first is applied with -Np1 and the second is applied with -Np0.

Applying patch: /home/conda/recipe_root/config.patch
Applying patch: /home/conda/recipe_root/config.patch with args:
['-Np1', '-i', '/tmp/tmpjghs1w3l/config.patch.native', '--binary']
checking file config.mk
patching file config.mk
Applying patch: /home/conda/recipe_root/cmake.patch
Patch level ambiguous, selecting least deep
Applying patch: /home/conda/recipe_root/cmake.patch with args:
['-Np0', '-i', '/tmp/tmpm1422urg/cmake.patch.native', '--binary']

In the end, the build fails because CMakeLists.txt was not created in the expected place.

Expected Behavior

I would expect both patches to be applied with -p1.

Steps to Reproduce

Try supplying a patch that creates a new file.

Output of conda info
     active environment : base
    active env location : /opt/conda
            shell level : 1
       user config file : /home/conda/.condarc
 populated config files : /home/conda/.condarc
          conda version : 4.10.1
    conda-build version : 3.21.4
         python version : 3.8.10.final.0
       virtual packages : __linux=5.8.0=0
                          __glibc=2.12=0
                          __unix=0=0
                          __archspec=1=x86_64
       base environment : /opt/conda  (writable)
      conda av data dir : /opt/conda/etc/conda
  conda av metadata url : https://repo.anaconda.com/pkgs/main
           channel URLs : https://conda.anaconda.org/conda-forge/linux-64
                          https://conda.anaconda.org/conda-forge/noarch
                          https://repo.anaconda.com/pkgs/main/linux-64
                          https://repo.anaconda.com/pkgs/main/noarch
                          https://repo.anaconda.com/pkgs/r/linux-64
                          https://repo.anaconda.com/pkgs/r/noarch
          package cache : /opt/conda/pkgs
                          /home/conda/.conda/pkgs
       envs directories : /opt/conda/envs
                          /home/conda/.conda/envs
               platform : linux-64
             user-agent : conda/4.10.1 requests/2.25.1 CPython/3.8.10 Linux/5.8.0-1033-azure centos/6.10 glibc/2.12
                UID:GID : 1001:1001
             netrc file : None
           offline mode : False

@ltalirz
Copy link
Author

ltalirz commented Jun 25, 2021

I believe the code that does the guessing is

def _guess_patch_strip_level(filesstr, src_dir):
""" Determine the patch strip level automatically. """
maxlevel = None
files = {filestr.encode(errors='ignore') for filestr in filesstr}
src_dir = src_dir.encode(errors='ignore')
guessed = False
for file in files:
numslash = file.count(b'/')
maxlevel = numslash if maxlevel is None else min(maxlevel, numslash)
if maxlevel == 0:
patchlevel = 0
else:
histo = {i: 0 for i in range(maxlevel + 1)}
for file in files:
parts = file.split(b'/')
for level in range(maxlevel + 1):
if os.path.exists(join(src_dir, *parts[-len(parts) + level:])):
histo[level] += 1
order = sorted(histo, key=histo.get, reverse=True)
if histo[order[0]] == histo[order[1]]:
print("Patch level ambiguous, selecting least deep")
guessed = True
patchlevel = min([key for key, value
in histo.items() if value == histo[order[0]]])
return patchlevel, guessed

It may be confused by the fact that the file does not (yet) exist.
I'm not familiar with the use case of the -p0 fallback, so I'm not sure whether one can really fix this
See also #737

P.S. Perhaps I am also making some mistake here; it seems strange to me that this issue would not have been noticed before. I just don't see what my mistake is.

@github-actions
Copy link

Hi there, thank you for your contribution!

This issue has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further activity occurs.

If you would like this issue to remain open please:

  1. Verify that you can still reproduce the issue at hand
  2. Comment that the issue is still reproducible and include:
    - What OS and version you reproduced the issue on
    - What steps you followed to reproduce the issue

NOTE: If this issue was closed prematurely, please leave a comment.

Thanks!

@github-actions github-actions bot added the stale [bot] marked as stale due to inactivity label May 27, 2023
@github-actions github-actions bot added the stale::closed [bot] closed after being marked as stale label Jun 26, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 26, 2023
@github-project-automation github-project-automation bot moved this to 🏁 Done in 🧭 Planning Jun 26, 2023
@sdebionne
Copy link

sdebionne commented Aug 23, 2023

Latest versions of conda/conda-build still have this issue with patches containing new files:

Extracting download
Applying patch: ~/conda-forge/staged-recipes/recipes/tinycbor/0001-Add-CMake-support.patch
Patch level ambiguous, selecting least deep
Applying patch: ~/conda-forge/staged-recipes/recipes/tinycbor/0001-Add-CMake-support.patch with args:
['-Np0', '-i', '/tmp/tmpu6ts3xxe/0001-Add-CMake-support.patch.native', '--binary']

with

# Name                    Version                   Build  Channel
conda                     23.3.1           py38h578d9bd_0    conda-forge
conda-build               3.25.0           py38h578d9bd_0    conda-forge

-p0? What on Earth?

@jezdez jezdez reopened this Aug 24, 2023
@github-project-automation github-project-automation bot moved this from 🏁 Done to 🏗️ In Progress in 🧭 Planning Aug 24, 2023
@jezdez jezdez added stale::recovered [bot] recovered after being marked as stale and removed stale::closed [bot] closed after being marked as stale labels Aug 24, 2023
mencian pushed a commit to bioconda/bioconda-recipes that referenced this issue Oct 10, 2023
* Enable macOS builds for AnchorWave

* Bump build number and add run_exports for Anchorwave

* Use lowercase name for AnchorWave in metadata

* Patch in upstream macOS-x86 support

* Patch CMakeLists.txt directly

Because of conda/conda-build#4241, new files aren't properly patched
in with conda-build, so patch the existing CMakeLists.txt instead of
adding a new one and symlinking.
@andrsd
Copy link

andrsd commented Jun 19, 2024

I think I am running into the same problem as @sdebionne. I have a patch that only adds files, so it looks like this:

diff --git a/CMakeLists.txt b/CMakeLists.txt
new file mode 100644
index 0000000..2a081e3
--- /dev/null
+++ b/CMakeLists.txt

The code in _guess_patch_strip_level does this:

        for patch in patches:
            parts = patch.parts
            for level in range(maxlevel + 1):
                if Path(src_dir, *parts[-len(parts) + level :]).exists():
                    histo[level] += 1
        order = sorted(histo, key=histo.get, reverse=True)
        if histo[order[0]] == histo[order[1]]:
            print("Patch level ambiguous, selecting least deep")
            guessed = True

The if Path(src_dir, *parts[-len(parts) + level :]).exists(): is never True, because the tested files do not exists. Then, we end up with histo[order[0]] == histo[order[1]] and we see the "Patch level ambiguous" message.

However, in _get_patch_attributes, there is code that detects if the patch was produced by git:

    files_list, is_git_format = _get_patch_file_details(path)
    files = set(files_list)
    amalgamated = False
    if len(files_list) != len(files):
        amalgamated = True
    strip_level, strip_level_guessed = _guess_patch_strip_level(files, src_dir)
    if strip_level:
        files = {f.split("/", strip_level)[-1] for f in files}

So, if I use the is_git_format information and change the code like so:

diff --git a/conda_build/source.py b/conda_build/source.py
index c8d21a4c..a62ce078 100644
--- a/conda_build/source.py
+++ b/conda_build/source.py
@@ -754,7 +754,11 @@ def _get_patch_attributes(
     amalgamated = False
     if len(files_list) != len(files):
         amalgamated = True
-    strip_level, strip_level_guessed = _guess_patch_strip_level(files, src_dir)
+    if is_git_format:
+        strip_level = 1
+        strip_level_guessed = False
+    else:
+        strip_level, strip_level_guessed = _guess_patch_strip_level(files, src_dir)
     if strip_level:
         files = {f.split("/", strip_level)[-1] for f in files}

my patches get applied correctly. Would this be a something we could do to fix this problem (even though it is not a perfect solution)? Or is there a scenario when git would produce patches that do not have patch level 1?

andrsd added a commit to andrsd/conda-build that referenced this issue Sep 13, 2024
@andrsd andrsd linked a pull request Sep 13, 2024 that will close this issue
3 tasks
@timoffex
Copy link

timoffex commented Oct 16, 2024

Just ran into this while debugging why my patch didn't work in conda-forge/wandb-feedstock#132 (commit: conda-forge/wandb-feedstock@3c716b5). Is there a workaround until the PR is merged?

EDIT: I worked around this by modifying a random file in my patch. Note that conda-build determines the patch level as "ambiguous" in this situation, so I could be getting lucky that 1 is preferred over 0 in my case:

Patch analysis gives:
[[ RA-MD1LOVE ]] - [[                                   01-find_nvidia_stats_executable.patch ]]
[[ RA-MD1-OVE ]] - [[                                              02-top_level_go_file.patch ]]

Key:

R :: Reversible                       A :: Applicable
Y :: Build-prefix patch in use        M :: Minimal, non-amalgamated
D :: Dry-runnable                     N :: Patch level (1 is preferred)
L :: Patch level not-ambiguous        O :: Patch applies without offsets
V :: Patch applies without fuzz       E :: Patch applies without emitting to stderr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale::recovered [bot] recovered after being marked as stale stale [bot] marked as stale due to inactivity
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants