gccrs icon indicating copy to clipboard operation
gccrs copied to clipboard

Check for writes outside of the build directory

Open powerboat9 opened this issue 1 year ago • 6 comments

I noticed that libgrust/libformat_parser/target was seemingly generated outside the build directory on my machine. This should detect similar issues, and confirm/deny the aforementioned issue.

powerboat9 avatar Apr 25 '24 17:04 powerboat9

@powerboat9 The MacOS CI broke, we repaired it but you need to rebase your branch now.

P-E-P avatar May 06 '24 12:05 P-E-P

That specific issue was addressed by #2947 "Move 'libformat_parser' build into the GCC build directory, and into libgrust" -- but yes, good idea to actually make sure we're not introducing similar things again! :+1:

tschwinge avatar May 07 '24 15:05 tschwinge

Eh, the build now actually is failing due to a similar issue:

cargo build --manifest-path ../../gcc/rust/checks/errors/borrowck/ffi-polonius/Cargo.toml --release --target-dir rust/ffi-polonius
    Updating crates.io index
error: failed to write /home/runner/work/gccrs/gccrs/gcc/rust/checks/errors/borrowck/ffi-polonius/Cargo.lock

...., so that'll need to be addressed first (in a similar way as #2947 "Move 'libformat_parser' build into the GCC build directory, and into libgrust", I suppose).

tschwinge avatar May 07 '24 15:05 tschwinge

From what I see from man documentation the chmod command doesn't accept flags after the mode(s) are given, but I'll change it just in case. Good catch though

powerboat9 avatar May 07 '24 17:05 powerboat9

It's easy enough to try:

$ ls -l
total 0
-rw-r--r-- 1 thomas thomas 0 May  8 10:50 --verbose
-rw-r--r-- 1 thomas thomas 0 May  8 10:50 foo
$ chmod -R a-w *
mode of 'foo' changed from 0644 (rw-r--r--) to 0444 (r--r--r--)
$ ls -l
total 0
-rw-r--r-- 1 thomas thomas 0 May  8 10:50 --verbose
-r--r--r-- 1 thomas thomas 0 May  8 10:50 foo

tschwinge avatar May 08 '24 08:05 tschwinge

Looks like cargo is trying to update the lock file and fail. I've noticed ffi-polonius does not use the same cargo config as libformat parser. Maybe we should update the cargo invocation with a --config argument ?

P-E-P avatar Jul 16 '24 16:07 P-E-P

I think it's worth passing some arguments to be safe. Even better would be to never call cargo directly and always through a variable to make sure we don't drop args.

In Gentoo, in cargo.eclass, we do:

cat > "${ECARGO_HOME}/config.toml" <<- _EOF_ || die "Failed to create cargo config"
[source.gentoo]
directory = "${ECARGO_VENDOR}"

[source.crates-io]
replace-with = "gentoo"
local-registry = "/nonexistent"

[net]
offline = true

[build]
jobs = $(makeopts_jobs)
incremental = false

[term]
verbose = true
$([[ "${NOCOLOR}" = true || "${NOCOLOR}" = yes ]] && echo "color = 'never'")
$(_cargo_gen_git_config)
_EOF_

export CARGO_HOME="${ECARGO_HOME}"

You don't need the local registry bit (although it doesn't do any harm), but you get the idea wrt offline. We also always invoke cargo with --offline. We could also set CARGO_HOME to be a temporary directory within the builddir.

thesamesam avatar Sep 30 '24 23:09 thesamesam

It looks like the issues preventing this PR from passing checks have since been fixed

powerboat9 avatar Oct 05 '24 04:10 powerboat9