Coder Social home page Coder Social logo

Comments (14)

jprjr avatar jprjr commented on August 22, 2024

Some time ago I switched to GraphicsMagick because of this - it has a way more sane CLI.

You basically take any ImageMagick command and prefix it with gm, eg:

gm import /path/to/image

gm convert src.png dest.jpg

and so on.

from s6-overlay.

glerchundi avatar glerchundi commented on August 22, 2024

Although I agree with @jprjr, we need to tackle this problem in order to avoid future collisions.

There is a similar issue in alpine linux bug tracker, https://bugs.alpinelinux.org/issues/4504. Maybe we need to take the same approach as debian takes.

from s6-overlay.

sameersbn avatar sameersbn commented on August 22, 2024

bit by the same 🐛

from s6-overlay.

pubyun avatar pubyun commented on August 22, 2024

same issue

from s6-overlay.

bear0330 avatar bear0330 commented on August 22, 2024

+1

from s6-overlay.

jprjr avatar jprjr commented on August 22, 2024

After reading the Alpine bug report I have an idea for tackling this:

We stop using import and use importas instead, and remove /usr/bin/import from the tarball

  • import is basically a slightly easier wrapper around importas anyways.
  • This can break existing user scripts, but it's a fairly straightforward fix on their end.
  • Zero conflicts with ImageMagick

We'd have to bump this up as a major version and make it really, really clear in the release notes that upgrading means you have to edit your scripts.

What do you think @glerchundi @skarnet ?

I'm going to go ahead and replace the import calls with importas anyways, the remaining question is should we remove /usr/bin/import from the tarball? I know some package managers will complain if a user tries to install ImageMagick if there's an existing /usr/bin/import

from s6-overlay.

glerchundi avatar glerchundi commented on August 22, 2024

Hi @jprjr, i think this is ok as a temporary fix but the problem still remains until we do one of the following:

  1. Use a custom folder for execline binaries (/usr/lib/execline/bin, not for execlineb which will be placed in /usr/bin as it's where it's right now.) and make them available when execlineb scripts are executed. In the way the bug report proposes. This would solve all kind of issues like calling import from bash scripts (which i think is why people got bitten by this) and it can be easily work arounded in execline by using absolute paths.
  2. Remove import from binaries

As you can guess, I would bet for option 1 ;)

from s6-overlay.

jprjr avatar jprjr commented on August 22, 2024

Hi @glerchundi, I think option 1 makes sense, we'd want to make sure /usr/lib/execline/bin is added to the end of the PATH, so that import from ImageMagick takes precedence if/when it gets installed (I'm sure there's some program out there that relies on it).

I do think it would generally be a good idea to use importas over import, since that avoids a name conflict entirely. I just spot-checked a few different distros, none of them have a program named importas in any of their packages (unless it's an execline package :) )

from s6-overlay.

pubyun avatar pubyun commented on August 22, 2024

prefer option 1.

there may be other conflicts later, and we delete or rename the program later?

from s6-overlay.

skarnet avatar skarnet commented on August 22, 2024

Option 1 will break s6. For some tools in the s6 distribution to work, all execline binaries need to be accessible via a PATH search, even without an execlineb invocation. The Debian "fix", and the suggested Alpine fix, are not fixes, they're ugly workarounds that create more hidden problems.

A real solution to the name conflict problem is to have a package manager system that, unlike FHS, can handle such conflicts. Slashpackage is one of them. If you are using a slashpackage installation for skalibs|execline|s6|... then you can use the suggested workaround and it won't break anything because s6 will hardcode the slashpackage absolute paths to execline's import binary, and /usr/bin/import will be ImageMagick's without confusion.

When you cannot use slashpackage, separate installation paths can be used as a substitute. In current Alpine edge (which Alpine 3.3 will be cut out of in a few days), execline binaries now reside in /bin, so /bin/import is execline's and /usr/bin/import is ImageMagick's. /bin can be set either before or after /usr/bin in PATH, depending on the wanted meaning for import. It's not very satisfactory, but it's better than nothing.

I would suggest to upgrade to Alpine edge and use /usr/bin/import systematically when you want to call ImageMagick's import. The overlay scripts could also be patched to only use importas as @jprjr suggested, which would fix the immediate issue of containers not starting.

from s6-overlay.

glerchundi avatar glerchundi commented on August 22, 2024

A real solution to the name conflict problem is to have a package manager system that, unlike FHS, can handle such conflicts. Slashpackage is one of them. If you are using a slashpackage installation for skalibs|execline|s6|... then you can use the suggested workaround and it won't break anything because s6 will hardcode the slashpackage absolute paths to execline's import binary, and /usr/bin/import will be ImageMagick's without confusion.

I didn't use /command because I feel it was ugly, of course in my naivete. Now I spent a little bit more time reading about djb's slashpackage, I think it was a very bad design decision to use /usr/bin (at least for most of them) for skarnet's software.

I would suggest to upgrade to Alpine edge and use /usr/bin/import systematically when you want to call ImageMagick's import. The overlay scripts could also be patched to only use importas as @jprjr suggested, which would fix the immediate issue of containers not starting.

Bitten by that decision, now depending on the order you place your binaries in your container import will point to execline's or imagemagick, which is weird, confusing and error-prone.

from s6-overlay.

glerchundi avatar glerchundi commented on August 22, 2024

@smerrill PTAL and confirm if it works for you. (Sorry for closing this, i didn't know that github automatically closes the issues you references in the commit messages).

from s6-overlay.

deminngi avatar deminngi commented on August 22, 2024

BTW I tested it and this error disappeared, @smerrill pls confirm my observation

from s6-overlay.

glerchundi avatar glerchundi commented on August 22, 2024

i tested it and it works, closing this.

from s6-overlay.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.