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

Regenerate golden images to make file paths compatible with go get #357

Merged

Conversation

AttilaTheFun
Copy link
Contributor

This PR fixes #353
I previously landed a PR to normalize the golden image file paths (replacing unsupported characters with hyphens), but CI didn't automatically regenerate the images.

This PR deletes all of the golden images and then regenerates them with valid file paths.

I ran this locally on my M1 Max MacBook Pro with the current homebrew vips release, 8.14.2_1.

I also tried to run the build/build.sh script as well to generate the linux versions with docker, but this seems to rely on one of
@tonimelisma 's private packages so it failed:

logan@Logans-MBP govips % cd build 
logan@Logans-MBP build % sh build.sh 
[+] Building 2.4s (10/17)                                                                                                       
 => [internal] load build definition from Dockerfile-ubuntu-18.04                                                          0.0s
 => => transferring dockerfile: 1.13kB                                                                                     0.0s
 => [internal] load .dockerignore                                                                                          0.0s
 => => transferring context: 2B                                                                                            0.0s
 => [internal] load metadata for docker.io/library/ubuntu:18.04                                                            0.8s
 => [auth] library/ubuntu:pull token for registry-1.docker.io                                                              0.0s
 => [ 1/13] FROM docker.io/library/ubuntu:18.04@sha256:8aa9c2798215f99544d1ce7439ea9c3a6dfd82de607da1cec3a8a2fae005931b    0.0s
 => CACHED [ 2/13] RUN sed -e '/^#\sdeb-src /s/^# *//;t;d' "/etc/apt/sources.list"     >> "/etc/apt/sources.list.d/ubuntu  0.0s
 => CACHED [ 3/13] RUN apt-get update                                                                                      0.0s
 => CACHED [ 4/13] RUN apt-get -y --no-install-recommends install software-properties-common gpg-agent                     0.0s
 => CACHED [ 5/13] RUN apt-get -y --no-install-recommends install     build-essential devscripts lsb-release dput wget gi  0.0s
 => ERROR [ 6/13] RUN add-apt-repository -y ppa:tonimelisma/ppa                                                            1.6s
------
 > [ 6/13] RUN add-apt-repository -y ppa:tonimelisma/ppa:
#0 1.522 Cannot add PPA: 'ppa:~tonimelisma/ubuntu/ppa'.
#0 1.522 The user named '~tonimelisma' has no PPA named 'ubuntu/ppa'
#0 1.522 Please choose from the following available PPAs:
------

@coveralls
Copy link

Coverage Status

Coverage: 75.424% (-0.08%) from 75.501% when pulling 1a95389 on AttilaTheFun:lshire-regenerate-images into d54e310 on davidbyttow:master.

@tonimelisma
Copy link
Collaborator

Yeah, I should remove the docker images as they're for older Ubuntu versions and no longer relevant. The PPA was needed for older Ubuntus without recent libvips. but there's an LTS now with a current libvips so it's not necessary

Copy link
Collaborator

@tonimelisma tonimelisma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI likes your pictures (or uses different library versions)

@tonimelisma tonimelisma merged commit c6838fc into davidbyttow:master May 4, 2023
2 checks passed
@sonu27
Copy link

sonu27 commented May 5, 2023

@AttilaTheFun thanks for this, go get fixed. Curious to know why it would be breaking go get?

@AttilaTheFun
Copy link
Contributor Author

@sonu27 Go get fails if the filenames have colons in them. @tonimelisma accidentally broke this when adding the vips version which contains a timestamp to the file names.

I updated the logic to sanitize the file names, replacing those characters with hyphens. That didn't automatically update the images, though, so I had to delete and regenerate them.

@NatashaMitchko
Copy link

@AttilaTheFun Thanks for the fix! It's appreciated.

I am still seeing the error in for v2.12.0 via go get -u github.com/davidbyttow/govips/v2/vips@v2.12.0, is it possible there needs to be a new release with your change in it? I'm not sure who to ask about that

$ go get -u github.com/davidbyttow/govips/v2/vips@v2.12.0
go: downloading github.com/davidbyttow/govips/v2 v2.12.0
go get github.com/davidbyttow/govips/v2/vips@v2.12.0: create zip: resources/avif-8bit.Export_AVIF_8_Bit-linux-jammy_amd64_libvips-8.12.1-Wed Feb  2 14:43:28 UTC 2022.golden.avif: malformed file path "resources/avif-8bit.Export_AVIF_8_Bit-linux-jammy_amd64_libvips-8.12.1-Wed Feb  2 14:43:28 UTC 2022.golden.avif": invalid char ':'
resources/avif-8bit.Export_AVIF_8_Bit-linux-kinetic_amd64_libvips-8.13.0-Tue Aug 23 04:05:08 UTC 2022.golden.avif: malformed file path "resources/avif-8bit.Export_AVIF_8_Bit-linux-kinetic_amd64_libvips-8.13.0-Tue Aug 23 04:05:08 UTC 2022.golden.avif": invalid char ':'
resources/bmp.Decode_BMP-linux-jammy_amd64_libvips-8.12.1-Wed Feb  2 14:43:28 UTC 2022.golden.png: malformed file path "resources/bmp.Decode_BMP-linux-jammy_amd64_libvips-8.12.1-Wed Feb  2 14:43:28 UTC 2022.golden.png": invalid char ':'

etc.

@AttilaTheFun
Copy link
Contributor Author

@NatashaMitchko You can use the fixed commit by replacing the version with the SHA of the commit at the head of master:

https://stackoverflow.com/questions/53682247/how-to-point-go-module-dependency-in-go-mod-to-a-latest-commit-in-a-repo

Once @tonimelisma / @davidbyttow publish a new version you'll be able to go get it normally.

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.

Resource images with timestamps in the name cause invalid file path errors
5 participants