Add sanitizers

[trws]

What it says on the tin, adding support for using sanitizers so we can leverage those to find issues with fluxion. Also adds a first copy of a CMakePresets.json file so we can have a common set of known build and test configurations that we can all share. For now this is just default and asan, but I think others could be worthwhile.

[trws]

Ok, could use some guidance on dealing with the commit checking @grondo, @garlick. I used git subtree add to pull in the external so we'd have a base we can easily update with git later, but this generates a merge commit to pull in their sources. This is not an internal merge or anything else, is this a fundamental problem or is there a way we can mark this particular merge commit "ok"?

[grondo]

I haven't used git subtree so I'm not going to be much help. Our strategy has been to just copy code from elsewhere and add it in a single commit. It sounds like this is what git subtree merge --squash would do?

Anyway, the main purpose of the 'no merge commit' check is to avoid messy history due to some PRs including merge commits from merging master instead of just rebasing. That doesn't apply here, so if the merge commit is useful in some way, we should extend the pr-validator (which is just a very simple script) to allow that.

Also, we can have someone just manually merge this one by clicking "Merge without waiting for requirements to be met". But if git subtree is going to used for things like this commonly in flux-sched, you might want to tweak the validator script.

[trws]

Yup, that's exactly what I did was use git subtree add <path> <uri> --squash and yes that's what happens is it generates a single commit that contains all the other repository's code and then merges it into the history of the project. It's not something I expect to be terribly common, but it's a nice way to do this kind of thing and have git manage the provenance and updates rather than having to do something like unpack a tarball and add the contents or git archive | tar x. As it sits, this and maybe fmt were the only things I thought I might pull in that way, so we might be alright as is if manual merge is alright for a couple of rare one-offs.

[grondo]

What's weird is that there's the merge commit and then a Squashed content ... commit that seems to confuse github.. I guess that prior commit is just informational?

[grondo]

@trws: I was going to extend the pr-validator to allow git subtree merge commits, but I ran into a bit of a problem. I tried to rebase this branch on current master, and it failed spectacularly:

$ git rebase -i --autosquash origin/master
CONFLICT (add/add): Merge conflict in README.md
Auto-merging README.md
CONFLICT (add/add): Merge conflict in LICENSE
Auto-merging LICENSE
CONFLICT (add/add): Merge conflict in CMakeLists.txt
Auto-merging CMakeLists.txt
CONFLICT (add/add): Merge conflict in .gitignore
Auto-merging .gitignore
error: could not apply 4d93ce74... Squashed 'cmake/sanitizers-cmake/' content from commit 3f0542e4
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 4d93ce74... Squashed 'cmake/sanitizers-cmake/' content from commit 3f0542e4

It appears that use of subtrees breaks the ability to rebase? Since we use mergify to rebase this might break our current strategy for auto merging PRs.

None of the solutions in this SO post sound very pleasant to me.

I'm probably missing something really obvious here...

[grondo]

Eh, I guess you'd only ever need to rebase the subtree merge once, so if we manually press the merge button once it wouldn't be an issue after that.

[trws]

I think what's going on is that this is based around git's concept of merging unrelated histories, so there is a bit of care needed there. FWIW though, the --preserve-merges option mentioned in the article, now --rebase-merges, seems to rebase across it no problem:

vscode@7c880fb30e99:/workspaces/flux/flux-sched$ git rebase --rebase-merges upstream/master
Successfully rebased and updated refs/heads/add-sanitizers.

Assuming mergify plays nice it seems to work great, someone said they did a re-add afterward but I didn't have to do anything like that, it just worked straight-up. Is that enough for what you were hoping for?

I'm thinking it would be best to not push the rebase but let mergify do it (if we can) so we can ensure it handles it properly. If this causes problems, I'll import the manual way, just like the idea of having git aware of what I'm doing if possible.

[grondo]

Good point that perhaps mergify will do the right thing by default. I've opened a PR here: https://github.com/flux-framework/pr-validator/pull/6 that attempts to special case subtree commits. If we merge that then we can re-run the validate commits action and it should pass.

[trws]

Nicely done @grondo, green across the board. Feel OK approving?

[trws]

Dang, no dice with mergify. I'll rework this.

[trws]

Ok, no subtree this time. Should have the same effect, just a bit more annoying to update. Work for you @grondo?

[grondo]

Yeah, though I was fine with the subtree since we'd only have to rebase each time the subtree was merged.

I suggest adding the URL of the repository and the commit at which it was pulled in to the commit that adds cmake-sanitizers.

Edit: this is just to help out a future person that might want to update the vendored code.

[trws]

Good point. The mergify issue is what made me back off... maybe it would have been fine with a manual rebase, not sure. Actually, I'll try it, if that works we'd be better off for updating, and I'll add the details to the commit and the update script.

[trws]

Alright, ran into some hitches doing the rebase, but the subtree is back with extra notes and commands in the script for later use. Turns out forgetting the --strategy subtree on a rebase across one of these makes git cranky. Hopefully mergify will be more pleased this time.

[grondo]

It worked!