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.

Sign up for GitHub

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

icecc-create-env: add command line --addfile "path target" support #548

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

icecc-create-env: add command line --addfile "path target" support #548

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions client/icecc-create-env.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,14 @@ resolve_path()
# This could use realpath, but that's reportedly not that widely available.
convert_path_cdup ()
{
local filename="$1"
local directory=$(dirname $filename)
local fixed_directory=$(cd "$directory" >/dev/null && pwd)
echo ${fixed_directory}/$(basename $filename)
# Remove all /./ sequences.
local path=${1//\/.\//\/}

# Remove dir/.. sequences.
while [[ $path =~ ([^/][^/]*/\.\./) ]]; do
path=${path/${BASH_REMATCH[0]}/}
done
echo $path
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this part necessary?

Copy link
Author

Choose a reason for hiding this comment

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

I tested some cases where one might want to remap to a path in the compiler package that doesn't exist on the local system, and therefore the pwd approach would not work. This might be over-reaching now that I look at it again, because I can't think of legitimate reason to do this. Ideally you are trying to make the compiler package match the local system in such a case.

Copy link

Choose a reason for hiding this comment

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

Hm, I would prefer to not be doing this with Bash regexes if possible. They've always been a little odd in the way they work. I think we can just detect that the path doesn't exist here and therefore fail and require an absolute path? Would that be a good option, do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to not be doing this with Bash regexes if possible. They've always been a little odd in the way they work

dunno about the merit of the patch itself, but that argument seems weird to me. the script is written in bash, and we should use its full power when appropriate. there is nothing odd about these regexes; they just have non-posix syntax, as they had to be retro-fitted into the shell syntax.

Copy link

Choose a reason for hiding this comment

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

@ossilator True enough - the argument I was making was more about future readers than the writer. I've used Bash regexes many times before with success, but they tend to confuse people reading them in the future who aren't familiar with them.

In any case, I think overall I would prefer we use a different mechanism for adding paths that don't exist locally anyway and prefer that we use absolute paths with it unless we have a good reason not to.

}

add_file ()
Expand Down Expand Up @@ -485,7 +489,8 @@ if test -n "$clang"; then
# HACK: Clang4.0 and later access /proc/cpuinfo and report an error when they fail
# to find it, even if they use a fallback mechanism, making the error useless
# (at least in this case). Since the file is not really needed, create a fake one.
if test -d /proc; then
# Only add an empty file if the user did not attempt to add there own.
Copy link

Choose a reason for hiding this comment

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

I am a bit suspicious of the need to "include" files that don't exist on the source system to satisfy running a binary on a target system. I think we want a more explicit "create a fake file" mechanism.

if test -d /proc && [[ $extrafiles != *" /proc/cpuinfo"* ]]; then
mkdir $tempdir/fakeproc
mkdir $tempdir/fakeproc/proc
touch $tempdir/fakeproc/proc/cpuinfo
Expand All @@ -498,7 +503,9 @@ fi
save_stripprefix="$stripprefix"
stripprefix=
for extrafile in $extrafiles; do
add_file $extrafile
target=$(echo $extrafile | cut -d= -f1)
path=$(echo $extrafile | cut -d= -f2)
add_file $path $target
done
stripprefix="$save_stripprefix"

Expand Down