search

hendrikmuhs / ccache-action

github action to speedup building using ccache
MIT License
114 stars 52 forks source link

CCACHE_DIR environment variable is ignored by action #96

Closed n3world closed 1 month ago

n3world commented 2 years ago

If the CCACHE_DIR environment variable is set ccache will use that for its cache location but ccache-action is hardcoded to use .${ccacheVariant}. The action should either honor CCACHE_DIR or set it to the hard coded path.

jonashaag commented 2 years ago

I agree. Do you want to send a PR?

jonashaag commented 2 years ago

Maybe we can even stop overriding the cache dir. I’m not sure why we do it.

n3world commented 2 years ago

Maybe we can even stop overriding the cache dir. I’m not sure why we do it.

Without any context my guess is this is done to keep all files generated during the action in the workspace directory. It might be good to keep this behavior and override the environment variable by using the --dir argument to specify the directory in all calls to ccache.

Even though the cache_dir is set in the config that is overriden by the environment variable. https://ccache.dev/manual/latest.html#_configuration

hendrikmuhs commented 2 years ago

@n3world Whats the use case of overriding CCACHE_DIR? Was it set accidentally?

n3world commented 2 years ago

@n3world Whats the use case of overriding CCACHE_DIR? Was it set accidentally?

I do not think I know exactly what you are asking so I'll just explain the situation a bit more.

CCACHE_DIR is currently set in the environment in which this action runs. This action does not check the setting of that variable and always tries to use <workdir>/.ccache. ccache prefers environment variables over settings in the configuration file so when the variable is set ccache does not use the directory this action expects and this action does not work.

Two ways to resolve this would be this action honors the environment variable and puts everything there or it overrides it and uses the directory it wants to. I was suggesting to go with the later fix since that provides more isolation and less likely to have issues if multiple compilations are using the same filesystem, fighting over setting configuration values in the configuration file and cache bloat.

If you were actually asking why the environment variable is set in the first place I don't think that it is too important but for me it is part of the docker container environment. The container is used for development and there it is convenient to have one volume location for the ccache just for consistency in scripts across all users.

hendrikmuhs commented 2 years ago

Thanks @n3world, that makes it more clear to me. You explained it is a side effect of re-using a docker container for development and CI. That's what I meant with "accidental". I understand you are not asking for implementing the possibility to override CCACHE_DIR, but to fix the unexpected behavior you see. So there isn't a use case, there is "just" developer pain, spending time debugging why the actions isn't working the way it should.

However this is harder to fix than it seems. Afaik steps in a workflow are isolated. Removing CCACHE_DIR from the environment won't work, because it is only unset in the setup action step, compilation runs in its own step and CCACHE_DIR might just be there again. I guess you introduce it in your build scripts. At the moment I can only think of checking if CCACHE_DIR is set and either log a warning or even fail the action. But if CCACHE_DIR is set in build scripts I wonder if we are even able to detect it at all. Again the action finalizer runs in its own step and likely doesn't have CCACHE_DIR set. With other words: I am not sure the action can honor CCACHE_DIR even if we want to implement support.

@n3world Can you tell me where CCACHE_DIR is set in your case?

I suggest to add a troubleshooting section in the Readme.

n3world commented 2 years ago

However this is harder to fix than it seems.

I know in steps you can write to $GITHUB_ENV file to set an environment variable that are seen by subsequent steps. The same could be done in the setup action

But if CCACHE_DIR is set in build scripts I wonder if we are even able to detect it at all. Again the action finalizer runs in its own step and likely doesn't have CCACHE_DIR set. With other words: I am not sure the action can honor CCACHE_DIR even if we want to implement support.

If it is just set in the build script and not global this action couldn't do anything about that, other than just documenting that CCACHE_DIR being set breaks this action

@n3world Can you tell me where CCACHE_DIR is set in your case?

It is in the environment for the docker image so it is there for anything using that image.

krlmlr commented 11 months ago

Getting back to the question raised in https://github.com/hendrikmuhs/ccache-action/issues/96#issuecomment-1225258352: why do we need to override the cache directory?

In particular, storing it inside the worktree causes unintended consequences for actions that push back to the Git repository. If we must override, I propose to store it alongside the worktree.