search

mdolab / .github

0 stars 3 forks source link

Added tapenade checks #19

Closed ewu63 closed 3 years ago

ewu63 commented 3 years ago

Purpose

Closes #15. Supports both 3.10 and 3.16.

Type of change

What types of change is it? Select the appropriate type(s) that describe this PR

eirikurj commented 3 years ago

Older versions of tapenade do exist in their gitlab repo https://gitlab.inria.fr/tapenade/tapenade Cloning the repo and checking out the correct tag to recover the older versions should work. For us, the tap-3.10-5363 tag should be what we use. Should we not try to first integrate the older tapenade version, so we can add tests to the current committed code in our repos?

ewu63 commented 3 years ago

I tried this locally but could not get the older code to run, I got the error

Error: Could not find or load main class topLevel.Tapenade
Caused by: java.lang.ClassNotFoundException: topLevel.Tapenade

despite setting $TAPENADE_HOME and $JAVA_HOME. So I suspect that the sources have to be built, which I then tried to do following instructions here but did not get that to work, possibly due to incompatible Java versions.

At this point, I feel building from source is a bit too much hassle which is why I opted to just work with the new version. If they are okay with us re-distributing our own 3.10 binaries then we could just use that instead and it should work as done with 3.16 here.

eirikurj commented 3 years ago

Since tapenade 3.10 is effectively available, I think it's probably ok to self-host the 3.10 tar and use in our testing.

marcomangano commented 3 years ago

Hopefully we will completely switch to 3.16 in the next few weeks / months, so this is a temporary solution anyway

ewu63 commented 3 years ago

I have updated the PR with support for 3.10. Since we can't test this there will probably be things to fix once this is merged and we use this on a PR. I will probably just directly push to master to fix that then.

ewu63 commented 3 years ago 
1. Should we place this in a file that is separate from the style check? I agree it does not belong with the build or test scripts, but it seems like it may warrant a separate file as it is not exactly a style check.

Good point, I will move this to a separate check then. Can you remind me what exactly does this do? Obviously the file structure will be different in the .github repo but does this affect anything running these checks? Just the heading on the individual jobs or something?

2. I find that Tapenade uses Docker when running on both my Mac and Ubuntu machines. I am not sure if Tapenade can run without it. If it needs Docker while running, will this be an issue for us?

Docker is available on all these machines so I think we are okay here, again won't know until we try it.

bernardopacini commented 3 years ago 
1. Should we place this in a file that is separate from the style check? I agree it does not belong with the build or test scripts, but it seems like it may warrant a separate file as it is not exactly a style check.

Good point, I will move this to a separate check then. Can you remind me what exactly does this do? Obviously the file structure will be different in the .github repo but does this affect anything running these checks? Just the heading on the individual jobs or something?

Do you mean what exactly making a new file does? It will create a new block on Azure, just like the style and build / test ones. Other than that, it will look just like the others.

Another option is to simply make the "Style" name more general, such as "Code Consistency" or something along those lines and avoid a new file.

bernardopacini commented 3 years ago

These updates look good to me. I am not sure if in the long run we have any other checks that we can combine into the same file with Tapenade, in which case the Azure_Tapenade file might need a new name, but at least for what we need now this looks good to me.