seL4 / seL4-CAmkES-L4v-dockerfiles

Dockerfiles defining the dependencies required to build seL4, CAmkES, and L4v.
13 stars 40 forks source link

Delete obsolete clang symlink (instead of updating it to keep strange legacy) #46

Closed axel-h closed 2 years ago

axel-h commented 2 years ago

Actually, I wonder if the comment still holds today, that CAmkES has this card-coded. I could not find a a place where this is hard-coded and I've not see any errors with the wrong link either. This symlink came in with 9557742518be15676cac6369e061c83d94315882 from @tcptomato, but neither the commit message nor the PR https://github.com/seL4/seL4-CAmkES-L4v-dockerfiles/pull/14 mentions this part of the change. So, do we have to create this link at all?

tcptomato commented 2 years ago

I think that part was added by Luke. @LukeMondy, any insights?

lsf37 commented 2 years ago

It'd definitely be good to figure out if this is still needed. It'd be very weird for the compiler path to be hard coded.

lsf37 commented 2 years ago

According to grep, this repo is the only seL4 org repo that mentions /opt/clang (and only here), so chances are that this is no longer required. I'd be happy to remove it.

The commit is setting up stack, so there is a slight chance that something in stack was expecting clang under /opt/clang, but that also seems strange to me.

axel-h commented 2 years ago

@lsf37: OK, if you also can't make sense of it, I'm changing this PR to remove it.

The old change is still kept as https://github.com/Hensoldt-Cyber/seL4-CAmkES-L4v-dockerfiles/tree/patch-axel-1a

lsf37 commented 2 years ago

Hm, I wonder if the l4v image build failure is due to the different ghc version in the cache. I'll have a look at that.

lsf37 commented 2 years ago

Hm, I wonder if the l4v image build failure is due to the different ghc version in the cache. I'll have a look at that.

Ok, yes, it very likely is. I haven't updated the l4v image yet, so the tests are unaffected, but if you build it manually you either have to increase space for the docker disk or we need to update ghc in l4v as well.

LukeMondy commented 2 years ago

I think that part was added by Luke. @LukeMondy, any insights?

From memory, I also just transcribed it from whatever we had before..? I think some tool expected clang to be there... I think it was something Matt F wrote, which used clang to optimise something... Stretching my brain here :sweat_smile:

lsf37 commented 2 years ago

Ok, thanks for confirming. This has likely been standardised to use the normal compiler paths by now.

axel-h commented 2 years ago

Digging a bit mre, it was a1cd94ac6e57a27fb928bf84e5e4253f09381d68 ("Move to gcc5") that added this link back in 2016.

kent-mcleod commented 2 years ago

It was for https://github.com/seL4/pruner and https://github.com/seL4/camkes-tool/blob/master/tools/ckeywords.c. It could be removed if clang is already installed elsewhere now.

lsf37 commented 2 years ago

It was for https://github.com/seL4/pruner and https://github.com/seL4/camkes-tool/blob/master/tools/ckeywords.c. It could be removed if clang is already installed elsewhere now.

Ok, cool. I can confirm that there is /usr/bin/clang available on the PATH in the containers already.

lsf37 commented 2 years ago

@axel-h can you rebase? (I don't seem to have access to the branch) With the other two PRs now merged, the tests should pass.

axel-h commented 2 years ago

Almost working, seems the super linter wants a branch somewhere - maybe a feature to disable? Or is there no linter active for shell scripts and thus the changeset to process is empty?

You are not currently on a branch.
Please specify which branch you want to merge with.
See git-pull(1) for details.
    git pull <remote> <branch>
fatal: Invalid symmetric difference expression master...31ac917cc08ec8f7dbd3edcbb7cdccbdf9efb724
2022-06-24 14:49:05 [WARN]   No files were found in the GITHUB_WORKSPACE:[/github/workspace] to lint!
2022-06-24 14:49:05 [ERROR]   Failed to switch back to branch!
2022-06-24 14:49:05 [FATAL]   [fatal: reference is not a tree: 31ac917cc08ec8f7dbd3edcbb7cdccbdf9efb724]