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

Add warning to makepkg-mingw if mingw-w64-file/grep/sed are installed #3798

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cyrilarnould
Copy link

@cyrilarnould cyrilarnould commented May 21, 2023

Following the discussion in #14313, I've tried to implement a check in makepkg-mingw to remediate the issue a little bit.

Unfortunately, the pacman -Q query takes around 1.1 seconds (at least on my machine), I couldn't find a faster way to implement this however.

@Biswa96
Copy link
Member

Biswa96 commented May 21, 2023

Isn't it possible to check if the executable exists? For example, $MINGW_PREFIX/bin/file.exe.

@cyrilarnould
Copy link
Author

Yep, that seems like a better solution. I'll update the pull request.

@lazka
Copy link
Member

lazka commented May 23, 2023

do we have some specific evidence for each tool breaking something? Just so we have it documented somewhere.

@Biswa96
Copy link
Member

Biswa96 commented May 23, 2023

do we have some specific evidence for each tool breaking something?

This does not look something is breaking but suspicious. Here is a message while using ucrt64 file packaage.

==> Extracting sources...
Usage: F:\msys64\ucrt64\bin\file.exe [-bcCdEiklNnprsSvzZ0] [--apple] [--extension] [--mime-encoding]
            [--mime-type] [-e <testname>] [-F <separator>]  [-f <namefile>]
            [-m <magicfiles>] [-P <parameter=value>] [--exclude-quiet]
            <file> ...
       F:\msys64\ucrt64\bin\file.exe -C [-m <magicfiles>]
       F:\msys64\ucrt64\bin\file.exe [--help]

@cyrilarnould
Copy link
Author

cyrilarnould commented May 23, 2023

do we have some specific evidence for each tool breaking something? Just so we have it documented somewhere.

There's the discussion in #14313 which provides some examples of dangerous packages. I thought it best to only generate warnings for packages that are part of base-devel, seen as they don't have to be included in PKGBUILD dependencies.

As a concrete example, the UCRT64 build of mingw-w64-grep has the following issues:

  • Having mingw-w64-ucrt-x86_64-grep installed causes build() to hang indefinitely after the following output:
checking for ld used by gcc...
  • Having mingw-w64-ucrt-x86_64-sed installed causes the build to fail outright during prepare()after the following output:
autom4te-2.71: error: --trace=: no such file or directory
autoreconf-2.71: error: /usr/bin/autoconf-2.71 failed with exit status: 1
==> ERROR: A failure occurred in prepare().
    Aborting...
  • Having mingw-w64-ucrt-x86_64-file installed causes the following error message during extract():
Usage: C:\msys64\ucrt64\bin\file.exe [-bcCdEiklNnprsSvzZ0] [--apple] [--extension] [--mime-encoding]`
          [--mime-type] [-e <testname>] [-F <separator>]  [-f <namefile>]`
          [-m <magicfiles>] [-P <parameter=value>] [--exclude-quiet]`
          <file> ...`
     C:\msys64\ucrt64\bin\file.exe -C [-m <magicfiles>]`
     C:\msys64\ucrt64\bin\file.exe [--help]`

If preferable I can open an issue on Github of the above, for posterity.

Granted, mingw-w64-file seems to only cause a minor issue, and other than that the grep build succeeds. makepkg uses file to decide which decompression tool to use (see usr/share/makepkg/source/file.sh), but when mingw-w64-file fails, makepkg falls back to checking whether bsdtar can decompress it. As far as I can tell, bsdtar seems to be able to handle everything but I'm not too familiar with compression tools.

If this issue is deemed too small we can of course also remove the warning for mingw-w64-file. I am not aware of any other immediate problems with mingw-w64-file.

@cyrilarnould
Copy link
Author

I updated the PR to be a single commit (and also updated the revision)

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.

3 participants