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

Do not overwrite already optimized (same size) files #1308

Closed
ofrias opened this issue Dec 2, 2020 · 28 comments
Closed

Do not overwrite already optimized (same size) files #1308

ofrias opened this issue Dec 2, 2020 · 28 comments

Comments

@ofrias
Copy link

ofrias commented Dec 2, 2020

Hello.

When I run svgo repeatedly over the same SVG file it is always altered, even if the resultant file size is the same. This seems to happen because the internal IDs are modified.

I attach several files which exhibit this problem.

mutant svgs.zip

This issue creates a problem when the files are in version control systems, or when their hash is used for something (for example for browser caching).

Thanks!

@TrySound
Copy link
Member

Hi @ofrias Instead of running repeatedly manually please use multipass option. It allows to avoid issues with duplicated prefixes.

@ofrias
Copy link
Author

ofrias commented Feb 19, 2021

The problem can be reproduced even if you run svgo using multipass (which is what we do). This is not a workaround.

@TrySound
Copy link
Member

Did you check the latest version? I landed a fix in https://github.com/svg/svgo/releases/tag/v2.0.1

@ofrias
Copy link
Author

ofrias commented Feb 19, 2021

I am trying, but I get this error:

$ ./bin/svgo --help
internal/modules/cjs/loader.js:638
    throw err;
    ^

Error: Cannot find module 'commander'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)

I have already installed commander using npm ( sudo npm install commander -g ) but problem persists.

@TrySound
Copy link
Member

Run only npm i. There is no need for global dependencies in this repo.

@ofrias
Copy link
Author

ofrias commented Feb 19, 2021

Thanks. I have tried with version 2.0.2 and all the files in the zip included in the original report, except from 1 (gq.svg) exhibit the same problem. Every time you run svgo --multipass on them they end up with the same exact size but different contents due to differences in ids. Could you add a check to do not overwrite the original file if the compressed file has the same size?

@TrySound
Copy link
Member

I'm not sure it's possible without reading of existing file. Reading may be quite expensive operation.

@ofrias
Copy link
Author

ofrias commented Feb 19, 2021

Would it be possible then to generate the ids sequence always starting by the same number? Not doing this is what causes the file differences.

@TrySound
Copy link
Member

Could you clarify which ids are you talking about?

@ofrias
Copy link
Author

ofrias commented Feb 19, 2021

For example this file contains this tag:

<clipPath clipPathUnits="userSpaceOnUse" id="h">

The problem is that every time you run svgo on this file the id changes to a different value, first to id="o", then to id="n", etc.

@TrySound
Copy link
Member

Do you specify any config?

@ofrias
Copy link
Author

ofrias commented Feb 19, 2021

No, just svgo --multipass

@TrySound TrySound reopened this Feb 19, 2021
@polarathene
Copy link

Just to clarify, are the repeated runs on the same source or are you checking in the output into git and running against that in future runs?

I wasn't able to reproduce the mentioned id issue with the linked Volaris SVG, but did notice --multipass with a data URI type of unenc repeated the URI prefix: data:image/svg+xml,data:image/svg+xml, (v2.3).

@ofrias
Copy link
Author

ofrias commented Apr 16, 2021

The repeated runs are not on the same source. We run svgo against the output of previous runs. We have an images folder with svgs, on version control, and when we add new images we run svgo on the full folder to be sure that all images are optimized.

@polarathene
Copy link

@ofrias I figured as much :)

So the issue of the id changing doesn't occur for you if the source is unaltered? Is it only deterministic when the input SVG file is the same or does that still cause a difference to output when other SVG files have been added/removed/modified when running SVGO on the entire images folder?

I've only run against single SVGs personally, but perhaps the plugin is not resetting the counter it increments for generating IDs when it processes multiple SVG (although multipass on the same SVG made no difference for me either).

@ofrias
Copy link
Author

ofrias commented Apr 17, 2021

Here you have the full test sequence with this file. In this case after 3 executions the file ends up being exactly the same as the original one. It looks like it goes through a never ending cycle.

$ svgo --version
2.3.0

$ cut -c116-121 Volaris.svg 
id="h"

$ svgo --multipass Volaris.svg 
Volaris.svg:
Done in 36 ms!
4.271 KiB - 0% = 4.271 KiB

$ cut -c116-121 Volaris.svg 
id="o"

$ svgo --multipass Volaris.svg 
Volaris.svg:
Done in 35 ms!
4.271 KiB - 0% = 4.271 KiB

$ cut -c116-121 Volaris.svg 
id="n"

$ svgo --multipass Volaris.svg 
Volaris.svg:
Done in 36 ms!
4.271 KiB - 0% = 4.271 KiB

$ cut -c116-121 Volaris.svg 
id="h"

@polarathene
Copy link

I downloaded the zip, extracted the SVG into a temporary folder (downloads directory on my system, not /tmp), ran the same sequence of commands but got rather different output after running SVGO.

Turned out that it was running my svgo.config.js further up the directory tree. Thankfully I added a helpful console.log("custom config") that I noticed in the output of SVGO commands.

After taking care of that unexpected surprise of config crawling, I was getting deterministic results. id="h" was always the result.


Is it possible that there is some config file being read in accidentally for you as well? Perhaps try running your reproduction from another system or at least another directory higher up so that no config searching by SVGO is contributing to reproduction issues?

If that isn't the problem, it would be difficult to pin-point the issue if no one can reproduce it. I suppose the next step would be running SVGO within a docker container as a way to isolate to a more common environment.

@ofrias
Copy link
Author

ofrias commented Apr 17, 2021

In my case there is no svgo.config.js file up the tree. Has svgo dependencies on other npm packages? If this is the case, could you please share the versions of these other packages in your system? I am running it on Ubuntu 20.04, don't know if this can make any difference.

$ cat /etc/lsb-release 
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=20.04
DISTRIB_CODENAME=focal
DISTRIB_DESCRIPTION="Ubuntu 20.04.2 LTS"

@polarathene
Copy link

Are you familiar with Docker? It's pretty straight-forward to install, your distro should have a package for it.

With Docker installed run the following command via CLI from the directory with the original Volaris.svg extracted from the zip:

$ docker run --rm -it --volume "$PWD/Volaris.svg:/svg/Volaris.svg" --workdir "/svg" node:alpine ash

$ yarn global add svgo

$ cut -c116-121 Volaris.svg
id="h"

$ svgo --multipass Volaris.svg
Volaris.svg:
Done in 49 ms!
4.271 KiB - 0% = 4.271 KiB

$ cut -c116-121 Volaris.svg
id="h"

$ exit

You should be able to reproduce that same output this way. It runs SVGO from within a containerized OS (Official NodeJS installed on Alpine Linux in this case), similar to a Virtual Machine if that helps.

With the exit command it will leave the container shell ash we ran with the Docker container, all state will be reset/deleted (The container will remove itself (--rm)), except for the volume mounted (--volume) Volaris.svg file from the host filesystem.

That might help you isolate the problem from your system as something doesn't seem right on it?


Has svgo dependencies on other npm packages? If this is the case, could you please share the versions of these other packages in your system?

Are you running this within a project directory? Did you try in an isolated location instead? The Docker test above should help verify that your system environment is affecting the results for some reason.

I am running it on Ubuntu 20.04, don't know if this can make any difference.

I'm doubtful of that causing a problem, but it may not be invalid (eg Alpine Linux sometimes has compatibility issues). One could verify this just by taking the ubuntu:20.04 DockerHub image and running the same commands as with the node:alpine example, but NodeJS must be installed inside the container first.

I am running Manjaro Linux myself.

@ofrias
Copy link
Author

ofrias commented Apr 19, 2021

I have reproduced your same resutls with Docker, so there is no issue in this particular Docker setup. But I am running a standard Ubuntu 20.04, without any special configuration, and I have this issue. I am speculating, but maybe there is a variable not initialized, or something similar, that exhibits this behavior on Ubuntu 20.04 and not in the Docker setup.

@ofrias
Copy link
Author

ofrias commented Apr 19, 2021

By the way, I can reproduce the same issue without using --multipass, so it is not related to this flag.

@ofrias
Copy link
Author

ofrias commented Apr 19, 2021

Forgot to answer that we are not running the test on a project directory (tried with /tmp/ for example)

We have even executed it on 2 different systems (both with Ubuntu 20.04), with the same results.

@polarathene
Copy link

But I am running a standard Ubuntu 20.04, without any special configuration, and I have this issue.

Then the issue is specific to your system if the Docker image cannot reproduce it, you need to identify what is different.

Did you try with an ubuntu:20.04 Docker image? That should provide an environment like your running. If you still cannot produce that, try with a regular VM install of Ubuntu with VirtualBox, add a shared folder to the host for just the SVG, and see if the problem occurs in the VM. If it doesn't then this strongly suggests something about your host systems is misconfigured...?


One workaround in the meantime would perhaps be to run SVGO through a Docker container when you need to. You can write a simple Dockerfile that extends a base image to provide SVGO by default, then instead of running a container with the command ash, you could just provide the svgo command and a directory to volume mount for processing.

@ofrias
Copy link
Author

ofrias commented Apr 19, 2021

I think that the issue is affecting all Ubuntu 20.04 users, not just my system. As I told you we have reproduced it in 2 different PCs, and by 2 different people.

We are already using a workaround, which is basically do not committing the files that we know have this issue.

@polarathene
Copy link

polarathene commented Apr 19, 2021

As I suggested, verifying via Ubuntu Docker image was probably sufficient. You're likely running the NodeJS package from Ubuntu? As 20.04 is a LTS release, it stayed with the 2018 release of NodeJS 10.x.x (with updates), 10.19.0 was released on Feb 2020, perhaps a system upgrade/update will get you something newer if that's the case.

Alternatively you can use the same version manager I use on my system (separate from the system package that I only have installed for the system to use with system packages, not for development). I use asdf with the nodejs plugin.

Here's a CLI snippet to test via a Docker image of Ubuntu 20.04. You can easily reproduce your issue by using asdf to switch between versions with this test case you've provided in this issue.

# Start a docker image based off of Ubuntu 20.04
$ docker run --rm -it --volume "$PWD/Volaris.svg:/svg/Volaris.svg" --workdir "/svg" ubuntu:20.04

# Multi-line copy/paste command, installs 'asdf' version manager with nodejs version of our choosing, updates PATH env var explicitly to avoid reloading shell, then installs svgo
$ apt update && apt install --yes curl git gnupg \
  && git clone https://github.com/asdf-vm/asdf.git ~/.asdf --branch v0.8.0 \
  && echo ". $HOME/.asdf/asdf.sh" >> .bashrc \
  && export PATH="/root/.asdf/shims:/root/.asdf/bin:/usr/local/sbin:~/.asdf/installs/nodejs/15.14.0/.npm/bin/:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" \
  && asdf plugin add nodejs \
  && asdf install nodejs 15.14.0 && asdf global nodejs 15.14.0 \
  && npm install -g svgo

# Run you command sequence here as copy/paste or individual commands
$  cut -c116-121 Volaris.svg \
  && svgo Volaris.svg \
  && cut -c116-121 Volaris.svg

You can change the NodeJS version by replacing the version number with whatever suites, no need to install svgo again, error will be possible to reproduce on NodeJS version 10.x.x, doesn't seem to be affected in future major releases.

asdf install nodejs <version> && asdf global nodejs <version> (You can see a list of versions with asdf list all nodejs)


So while the 2.0 release in Feb 2021 mentions needing 10.13.x or newer, in this case to resolve this issue (as it appears to be NodeJS related not specifically svgo, you need NodeJS 11.x.x or newer 👍

@ofrias
Copy link
Author

ofrias commented Apr 19, 2021

OK, thanks for your help. Then I understand that this is a bug of svgo on NodeJS 10.x.x.

In my system:

$ nodejs --version
v10.19.0

If this is the case we prefer to stick to the versions provided by the OS as much as possible, so for the moment we will continue using the workaround until Ubuntu upgrades to a newer version of NodeJS.

@polarathene
Copy link

I don't follow Ubuntu much, but I would expect it updated to newer 10.x.x versions at the very least...

You could get newer releases by using Ubuntu 20.10 or 21.04 from this month release. The next Ubuntu LTS to replace 20.04 will be one more year away at 22.04,that's quite a while a way. NodeJS is already at 15.x.x, 3 years of improvements.

You also have the option of using the official Snap release. But using Docker is perfectly valid too as shown above, there are official Docker images from NodeJS on DockerHub (debian and alpine based), you can still be conservative with an LTS release of NodeJS if you like.

There's also leveraging free CI services online like Github Actions. Configure a yaml file to tell Github to use Ubuntu 20.04 or newer with or without docker image to run nodejs (Github maintains a version they keep up to date for their Ubuntu images ensuring stability and security). Great for automated builds like this with SVGO whenever pushing to master or merging a PR.

@SethFalco
Copy link
Member

Not sure why, but indeed I can still reproduce the issue with Node.js v10.19.0 with the latest version of SVGO.

However, since that's no longer a supported version, I'll just close the issue. ^-^'

Thanks for debugging this meanwhile, and sorry it took so long to review this.

@SethFalco SethFalco closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 2023
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

No branches or pull requests

4 participants