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

configuration for optional (armbian tool) local ccache #464 #466

Closed
wants to merge 38 commits into from
Closed

configuration for optional (armbian tool) local ccache #464 #466

wants to merge 38 commits into from

Conversation

golfromeo-fr
Copy link
Contributor

proposal for enhancement #464

golfromeo-fr and others added 30 commits July 17, 2016 18:34
Added fswebcam-gc2035 package
Copy link

@vincent-legoll vincent-legoll left a comment

Choose a reason for hiding this comment

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

The other part of that change look good. Let's see what others say...

@@ -31,6 +31,12 @@ FORCE_CHECKOUT="yes" # ignore manual changes to source
BUILD_ALL="no" # cycle through available boards and make images or kernel/u-boot packages.
# set KERNEL_ONLY to "yes" or "no" to build all packages/all images

PRIVATE_CCACHE="no" # Use separate ccache directory located in build tree ($DEST/ccache)
# Can be used to avoid problems with ownershipt on $HOME/.ccache.
Copy link

Choose a reason for hiding this comment

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

ownership of

PRIVATE_CCACHE="no" # Use separate ccache directory located in build tree ($DEST/ccache)
# Can be used to avoid problems with ownershipt on $HOME/.ccache.
# Set to "yes" if you are using build script as non-root user with "sudo"
# note: to use a ramdisk for ccache, create first a symlink to $DEST/ccache

Choose a reason for hiding this comment

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

first create

# Can be used to avoid problems with ownershipt on $HOME/.ccache.
# Set to "yes" if you are using build script as non-root user with "sudo"
# note: to use a ramdisk for ccache, create first a symlink to $DEST/ccache
# Set to "no" check if $DEST/ccache exists and delete (directory/symlink)
Copy link

Choose a reason for hiding this comment

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

I'm not sure I understand. It will automagically delete $dest/ccache and then use $home/.ccache ? I'll let the user do the cleanup himself, eventually printing a message that "there's a trailing ccache directory at $dest/ccache" because he may still want to use it later or he made the mistake to change the variable, or whatever. The content may be worth hours of compilation...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not exactly, the default config for ccache is $HOME/.ccache (here it is PRIVATE_CCACHE="no")
with PRIVATE_CCACHE="yes", you can have a ccache folder limited to Armbian tool usage.

Armbian tool creates "root" owned files in $HOME/.ccache (this is somewhat ugly)
I was in trouble ccache-compiling other softwares as "non-root" user so I guess the Armbian tool should clean its own mess

Moreover, (with more changes) it would offer the possibility to create ccache per SoC for mass compilation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... and yes, it will delete $DEST/ccache if it exists when PRIVATE_CCACHE="no" to switch back to $HOME/.ccache

In my opinion, if a tool creates mess (root owned files in $HOME/.ccache) it is better to offer the option to deal/avoid the mess by design (and not by luck/goodwill/etc.) so having a separate folder for ccache is a good option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and btw, I am French :)

Choose a reason for hiding this comment

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

OK, I understand the "need" for that cleanup, even if a bit unfortunate...
But untangling the need for running as "root" would be a bigger task, so I retract my objection.
LGTM with the other small glitches fixed...

PS: I'm also french (from Alsace)

@zador-blood-stained
Copy link
Member

Implemented in 870a7ac
In addition your PR deletes documentation file that still needs to be here

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

Successfully merging this pull request may close these issues.

None yet

3 participants