Skip to content

New --ccache option to cache build artifacts#53

Open
ydirson wants to merge 1 commit into
masterfrom
ccache
Open

New --ccache option to cache build artifacts#53
ydirson wants to merge 1 commit into
masterfrom
ccache

Conversation

@ydirson

@ydirson ydirson commented Sep 19, 2025

Copy link
Copy Markdown

Quite necessary for upcoming rebuilds of the kernel package.

Includes a new PATH_PREPEND interface to the container.

Comment thread src/xcp_ng_dev/cli.py
help='Environment variables passed directly to '
f'{RUNNER} -e')
parser.add_argument('--ccache', action='store',
help="Use given directory as a cache for ccache")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would rather use a docker volume for that, unless there is a point to access the ccached data from outside a container?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I use it to check how well that works with CCACHE_DIR=... ccache -s. Also, that allows a caller to have its own management of the lifecycle of this cache like other standard directlries, whereas I understand a docker volume would be in docker/podman's own data store.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, maybe get the value from an environment variable, so we don't have to always pass it on the command line then.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That would sure mirror the behavior of ccache, which tends to use a single cache dir, though I've mostly used it separate cache dirs for building different packages. I guess both usages make sense, and I'm wary of having 2 ways of passing the info - but for the bitbake use-case I kind like the simplicity of just appending options with:

XCPNGDEV_BUILD_OPTS:append = " --ccache=${TOPDIR}/ccache/${PN}"

If we want to add an envvar, at first I wondered whether simply using CCACHE_DIR here would be a good idea, rather than introducing a brand new one.

OTOH CCACHE_DIR was mostly designed as a way to pass a parameter, transparently to the compiler CLI to avoid modifying a build system. In fact I'm a bit unsure of the use-case you have in mind - just making sure all invocations are reusing the cache by adding it in your .profile?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just to avoid passing it by hand and take advantage of the cache when rebuilding a package

@ydirson ydirson requested a review from a team December 12, 2025 11:36
Comment thread src/xcp_ng_dev/cli.py
os.makedirs(args.ccache, exist_ok=True)
docker_args += ["-v", f"{os.path.realpath(args.ccache)}:/home/builder/ccachedir",
"-e", "CCACHE_DIR=/home/builder/ccachedir",
"-e", "PATH_PREPEND=/usr/lib64/ccache",

@rzr rzr Dec 12, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

any reason to hardcode path "/usr/lib64/ccache" here ?
unsure it will be portable in all systems but this is fair enough

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Whereas it's usually added to PATH when we login, when running commands that way it is not, so we have to add it to benefit from the transparent compiler wrappers.
Now yes, hardcoding the path is rather hackish and will cause issues for aarch64. I'm not sure if we (want to) introspect the container image to determine it, so we can probably get away with a conditional depending on release and arch.

}

if [ -n "${PATH_PREPEND}" ]; then
PATH="${PATH_PREPEND}:${PATH}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

no export needed ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

nope - if it's already exported we're in big trouble anyway :)

(I've always wondered why so many textbook examples insist on using export on this variable 😉)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

At startup, bash automatically maps the environment variables as variables and marks them as exported. When a variable is modified inside the bash process, bash passes the new value as an environment variable to the sub-processes.

I still use export anyway—for clarity and as a habit, I guess.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The underlying issue is that there is a single syntax for 2 different things :)

Quite necessary for upcoming rebuilds of the kernel package.

Includes a new PATH_PREPEND interface to the container.

Signed-off-by: Yann Dirson <yann.dirson@vates.tech>
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.

3 participants