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

CLI-1404: Artifact: .gitignore file parsing issues #1800

Open
regilero opened this issue Sep 18, 2024 · 2 comments
Open

CLI-1404: Artifact: .gitignore file parsing issues #1800

regilero opened this issue Sep 18, 2024 · 2 comments
Labels

Comments

@regilero
Copy link

Hello, I'm coming to acli after the BLT EOL (acquia/blt#4736).
We used BLT for artifact generation for ACSF and are trying to use acli for the same task.

BLT had several tools to manage the final files present in the artifact, some of them common with acli (like the sanitize steps).
But the main exclusion stuff was done via a specific deploy-exclude.txt list (and even hooks), and then a specific .gitignore file put in the final artifact (so not the same .gitignore file than the one used to manage the project.

Here acli is using the project main .gitignore file (and only this one) to perform the first copy of files and directories into the temporary working folder. Without any way to add a specific .gitignore for the final artifact.
So it seems everything which is not handled by the sanitize tasks have to be excluded in this main .gitignore file (and we are trying to merge the blt files in this .gitignore).

Using this first exclusion of files before making the copy seems to work, as most project would exclude in this file the Drupal core files and all the work linked to composer. And acli would run composer in the temp directory to regenerate the missing files. This is documented as a "speed up" in the code. But I think it is also a way of excluding files from the artifact.

Our project contains folders that we want to track in git, but that we do not want in the final artifact (you can think of tests for example, or front related stuff). Not a problem, we can generate a .gitignore file just before the artifact generation, to fit the real exclusion list. Here a --ignore-dirty like in BLT would be helpful as the fact that acli wants to get a clean git status before artifact generation is a minor problem, but that's another subject.

Problem

The problem is that in acli the parsing of this .gitignore is done via Symfony Finder ignoreVCS() function, and this function is quite bad at parsing the .gitignore file.

A simple rule like this one would fail in the finder:

/docroot/*
!/docroot/themes/custom

But this one would work for the finder:

/docroot/*
!/docroot/themes/custom/*

While being wrong for git (the leading /* in the exception is wrong)

I also reported issues with symlinks which may be directly related to this ignoreVCS() function in #1799.

It is quite simple to detect a syntax problem in a .gitignore file while using git, as you can see quite fast which file are tracked or not.

Finding a .gitignore syntax which would be compatible for both the Symfony Finder and git is really harder. And it takes a quuite long time to generate artifacts. Currently I'm still trying to find why I'm lacking some css and js files in my artifact and why I still have a node_modules/ folder with 3 js files in it while having a clear node_modules line in my .gitignore file.

It is currently very hard to produce an artifact of the same quality than blt artifacts.

solutions ?

I would not be surprised by a removal of ignoreVCS() function in the Finder some day. I would be very surprised in this function could one day parse the .gitignore file with the exact same behaviour than the latest git version.
So I think trying to only use the Finder while making the first copy of files will always be a problem.
Or at least the ignoreVCS() part.
One solution would be to add a deploy-exclude.txt file, with specific exclusion rules, and a syntax only valid for acli ... but that's a new file, and it breaks compatibility for people who only relies on the .gitignore file currently.

acli is expecting a working git directory as the starting point of the artifact generation, so another solution would be to use git to get a very precise file list.

This command for example:

git ls-files --cached --others --exclude-standard

Using this you get a very precise list of files that git is managing, based on the .gitignore rule (and maybe also based on the forced commit, and on files which were added before changes on the .gitignore files, not sure for these cases). using this would be a more clear reason of not supporting the --ignore-dirty mode as now the current git statut would become really important for the artifact.

This solution would lack an easy way of excluding files while still tracking them on git (like the current one), but at least we could be sure that a file not tracked by git would never be present in the artifact (with the exception of files generated after that by composer install).

@regilero regilero added the bug Something isn't working label Sep 18, 2024
@github-actions github-actions bot changed the title Artifact: .gitignore file parsing issues CLI-1404: Artifact: .gitignore file parsing issues Sep 18, 2024
@regilero
Copy link
Author

Fun fact I was wrong on one point. The ignoreVCS() has evolved and it's now recursive, so it recognize .gitignore files that can be set in subdirectories.
This is worse for the process of making an artifact dedicated .gitignore file and using it just for the artifact generation, as this should be done for all .gitignore files in the project.

In my case the front devs added a special .gitignore file in the theme directory, with a valid behavior for a theme:

node_modules
*.css
*.js
!*.es6.js

Because in git you want to store pre-build stuff. But for the artifact we now have two problems:

  • the bad parsing of exceptions in the Finder function Finder: ignoreVCSIgnored() remove or document deviations with VCS results  symfony/symfony#58175 , as here git would not even try to scan for exceptions in node_modules folder (you need node_modules/* in git if you want to let exceptions in the folder), but for the Symfony Finder a file like node_modules/punycode/punycode.es6.js would be wrongly added in the artifact
  • if we generate css and js file before building thè artifact this .gitignore file would still prevent us from adding these files in the artifact

The main issue, I think, is things you want in git and the things you want in the artifact cannot be managed the same way. I want css and js files in the artifact, maybe not in my git.

For example you could avoid completlty the .gitignore exception when mirroring things, and remove all .gitignore files in the mirror. And then add a special .gitignore file in the artifact directory, where all artifact rules are centralized. That's what we had with BLT.
Here trying to predict the content of the artifact is a funky job.

@danepowell danepowell added support and removed bug Something isn't working labels Oct 1, 2024
@regilero
Copy link
Author

regilero commented Oct 9, 2024

interesting labelling choice. Can you explain why it's not a bug, maybe it's a feature?

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

No branches or pull requests

2 participants