Added support for replacing sandbox paths in build artifacts

[UebelAndre]

This change expands on the define_absolute_paths and replace_absolute_paths functionality which replace absolute paths to files within EXT_BUILD_DEPS with that environment variable to be expanded at a later time and introduces define_sandbox_paths and replace_sandbox_paths which more generally replaces absolute paths to the exec root with ${EXT_BUILD_ROOT}. In both cases, these variables are update at the beginning of each build and are otherwise invisible to foreign build systems.

[UebelAndre]

@jsharpe Hey, do you have any ideas as to where these permissions errors are coming from in the standalone build?

[jheaff1]

@jsharpe Hey, do you have any ideas as to where these permissions errors are coming from in the standalone build?

Does this help - #622 (comment)

[UebelAndre]

Does this help - #622 (comment)

Sorry, I should have acknowledged I read that. It's not clear to me where the permissions error comes from and why the file wouldn't exist, especially after it being in the result of find. The behavior here doesn't make sense to me but I also have very limited experience with the standalone behavior. I think I'd be fine with some chmod +w but I don't think we can get away with double checking that the file exists. That already feels like we're in a bad state.

[jheaff1]

Does this help - #622 (comment)

Sorry, I should have acknowledged I read that. It's not clear to me where the permissions error comes from and why the file wouldn't exist, especially after it being in the result of find. The behavior here doesn't make sense to me but I also have very limited experience with the standalone behavior. I think I'd be fine with some chmod +w but I don't think we can get away with double checking that the file exists. That already feels like we're in a bad state.

Completely agree. If a check for the existence of a file is required directly after the file was included in a find result, something very strange is going on

[UebelAndre]

@jheaff1 It's green! 😄

Perhaps you could do some testing for your out_data_dirs PR (#622)?

[jheaff1]

@jheaff1 It's green! 😄

Perhaps you could do some testing for your out_data_dirs PR (#622)?

Nice one! Yep will do. I have now realised that my PR#677 isn’t quite working as expected, so the out_data_dirs attribute may be required to use a Python toolchain built using rules_foreign_cc. Hopefully the out_data_dirs PR will be all green once I rebase off this branch

[UebelAndre]

@jheaff1 I think the fact that this passes with the examples means that the changes are good for #622 what do you think?

[jheaff1]

@jheaff1 I think the fact that this passes with the examples means that the changes are good for #622 what do you think?

Yep I agree. I suppose the cleanest progression of PRs would be:

1. PR to add apr and apr_utils to examples
2. PR to add SQLite to examples
3. PR to add subversion to examples (which includes the required changes provided in this PR (Added support for replacing sandbox paths in build artifacts #650)

That said, I’d be happy for this to be merged now and then repurpose #677 to add the python dependencies to the examples.

Of course I don’t manage this repository so it’s not really up to me 😁

[UebelAndre]

I think it's good to have changes associated with tests. There's likely a way to super optimize the commits but IMO it's not worth the paperwork for this one. The examples stand as the regression tests for the rules merged here and again, that's a good thing to me. Lets keep things moving forward and get some of these sweet new changes in 😄