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

docker: Do not ignore certain git folder for docker build #4827

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

mmacata
Copy link
Contributor

@mmacata mmacata commented Dec 10, 2024

For docker alpine build, this fix prevents the following error during GRASS GIS configure step:
checking for GRASS GIS headers commit date... fatal: bad object HEAD 2024-12-10T08:46:00+00:00

With this fix it correctly returns:
checking for GRASS GIS headers commit date... 2024-11-29T10:01:48+00:00

@github-actions github-actions bot added the docker Docker related label Dec 10, 2024
@echoix echoix changed the title Do not ignore certain git folder for docker build docker: Do not ignore certain git folder for docker build Dec 10, 2024
@echoix
Copy link
Member

echoix commented Dec 10, 2024

Can you confirm (not out of all reasonable doubts, but only probably) that the final image stage isn't really bigger now, ie, it includes the pack folder inside .git, since it isn't ignored anymore. Check that .git/objects/pack isn't leaked into the final stage of the image. The pack files can contain the vast majority of the git repo's size.

In both cases, it will be ok to merge.

@mmacata
Copy link
Contributor Author

mmacata commented Dec 11, 2024

Ah good point!

The alpine build uses multistage and I can confirm that the size is not increased.

Ubuntu also uses multistage-build so I strongly assume this is also fine.

The debian build is not using multistage, so I ran a local comparison and indeed, the size is increased by around the size which the .git/objects/pack has (3.29GB vs 2.93GB for osgeo/grass-gis:main-debian). The folder itself is not included in the final debian image though because of this line which unfortunately has no effect on the image size.

@echoix
Copy link
Member

echoix commented Dec 11, 2024

Ok. Fine with me since we know the limitations

You're right with the Debian images, these three lines indeed do nothing, as it is an extra layer that removes files, the files are still there. We should either clean up by copying stuff out of a layer, or more easily, always clean up (delete files) in the same step they were added.

@echoix
Copy link
Member

echoix commented Dec 11, 2024

@mmacata Also, did you have the chance to think about #3821 since the community sprint?

@echoix echoix merged commit 6b955b4 into OSGeo:main Dec 11, 2024
26 of 27 checks passed
@github-actions github-actions bot added this to the 8.5.0 milestone Dec 11, 2024
@mmacata
Copy link
Contributor Author

mmacata commented Dec 11, 2024

@mmacata Also, did you have the chance to think about #3821 since the community sprint?

Ups, I somehow lost sight of that, will do that in the next days!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker Docker related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants