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

Mirror install tree layout in build tree #1776

Closed
letmaik opened this issue Oct 19, 2020 · 6 comments
Closed

Mirror install tree layout in build tree #1776

letmaik opened this issue Oct 19, 2020 · 6 comments

Comments

@letmaik
Copy link
Member

letmaik commented Oct 19, 2020

This will simplify a bunch of scripts that have path detection logic for working against an install vs build/dev tree. A concrete example is the invocation of sandbox.sh and related arguments like --binary-dir.

@achamayou
Copy link
Member

A very cheap way to do that would be to create two symlinks in cmake:

bin -> .
lib -> .

This may or may not cause various instances of directory-traversal logic to have a fit of course.

@achamayou
Copy link
Member

@letmaik just to check, it this just sandbox.sh only, or is there more?

@letmaik
Copy link
Member Author

letmaik commented Oct 29, 2020

sandbox.sh and any script that calls it. I'm not aware of other cases.

@achamayou
Copy link
Member

Ok, so, @jumaffre pointed out that we could tell cmake to structure the build dir similarly to install: https://stackoverflow.com/questions/6594796/how-do-i-make-cmake-output-into-a-bin-dir/6595001#6595001

A couple of thoughts though:

  1. This is nearly as much code as we would save from sandbox.sh
  2. For it to work for projects that use CCF (and sandbox), they also need to pick it up. So they either need to not forget to add this boilerplate, or we preemptively set it in common.cmake and change their build layout whether they like it or not.

I am not convinced that this is a reasonable improvement over the status quo, especially because of 2. Keeping a moderate (<10loc) amount of logic in sandbox.sh so CCF users can stick with common cmake defaults seems acceptable to me.

@letmaik
Copy link
Member Author

letmaik commented Oct 29, 2020

I don't mind how sandbox.sh looks like, but I do mind how

// This logic allows to run tests easily from a CCF install or the CCF repository.
// Most of this will disappear once CCF's build folder uses the same layout as an install.
let CCF_SANDBOX_SCRIPT: string
const CCF_REPO_ROOT = path.join('..', '..', '..')
const CCF_BINARY_DIR = process.env.CCF_BINARY_DIR || path.join(CCF_REPO_ROOT, 'build')
if (path.basename(CCF_BINARY_DIR) === 'bin') {
// ccf install tree
CCF_SANDBOX_SCRIPT = path.join(CCF_BINARY_DIR, 'sandbox.sh')
} else {
// ccf repo tree
CCF_SANDBOX_SCRIPT = path.join(CCF_REPO_ROOT, 'tests', 'sandbox', 'sandbox.sh')
CCF_SANDBOX_ARGS.push('--binary-dir', CCF_BINARY_DIR)
}
looks like.

@achamayou
Copy link
Member

I don't this outweighs pushing a cruel and unusual restriction on build and install matching on all downstream projects. It's nice that we auto-compute this path for users of that sample rather than tell in the doc to set their $PATH when running a sample from the build/ directory, but it's not worth such an extensive change.

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

No branches or pull requests

2 participants