Skip to content

Additions to the workflow for each benchmark.#7

Merged
mattmccutchen-cci merged 6 commits into
mainfrom
workflow-alltypes-and-compile
Mar 15, 2021
Merged

Additions to the workflow for each benchmark.#7
mattmccutchen-cci merged 6 commits into
mainfrom
workflow-alltypes-and-compile

Conversation

@mattmccutchen-cci

@mattmccutchen-cci mattmccutchen-cci commented Mar 5, 2021

Copy link
Copy Markdown
Collaborator
  • Convert with -alltypes as well as without.

  • Compile both converted programs, ignoring failure for the -alltypes version (we can still view the compile errors in the logs).

Other improvements:

  • Since the workflow file contains so much repetitive code that the workflow format does not support factoring out, the file is now generated by a Python program, generate-workflow.py. Since it is easy to forget to re-run generate-workflow.py after changing it, the workflow stops at the beginning if it detects it is out of date with generate-workflow.py.

  • For consistency, use the Checked C compiler for the pre-conversion build of each benchmark as well as the post-conversion build. To avoid regressing the ability to convert vsftpd at all, a fix for the error that blocked vsftpd from building with the Checked C compiler even before conversion is included in this PR.

  • Improve some names and clean up some shell commands.

  • Try not to overload gamera.

- Convert with -alltypes as well as without.

- Compile both converted programs, ignoring failure for the -alltypes
  version (we can still view the compile errors in the logs).

Other minor improvements:

- Improve some names.

- convert_project is now directly executable and we don't have to change
  to its directory.

- Try not to overload gamera.
@mattmccutchen-cci

Copy link
Copy Markdown
Collaborator Author

Is it time to start using a script to generate the workflow file? Maybe in Python to be consistent with most of our other scripts, even if there might be more specialized languages that are arguably better suited to the task? Writing more short scripts to factor out command sequences (in this repository if we don't want to put them in the 3C repository) would help a little, but there's no need to do that if we generate the whole workflow file with a script anyway.

@mwhicks1

mwhicks1 commented Mar 8, 2021

Copy link
Copy Markdown
Owner

This makes a lot of sense to me. I think what you are saying is that different targets require different sequences of steps to make them amenable for conversion, and sometimes only particular conversion steps should be carried out. Thus, we should write a meta description that indicates these steps, and a single script to generate the actions file that executes them. Is that right?

@mattmccutchen-cci

mattmccutchen-cci commented Mar 8, 2021

Copy link
Copy Markdown
Collaborator Author

I think what you are saying is that different targets require different sequences of steps to make them amenable for conversion, and sometimes only particular conversion steps should be carried out.

Correct.

Thus, we should write a meta description that indicates these steps, and a single script to generate the actions file that executes them. Is that right?

Roughly. I don't think our scale is large enough yet that it makes sense to have a formal "meta description" separate from the script, though of course the whole script could be viewed as a meta description.

I'll go ahead and start on the script and see if it finds any mistakes in my manual copying and pasting. 🙂

@mattmccutchen-cci mattmccutchen-cci marked this pull request as draft March 8, 2021 15:21
The program mostly reproduces the existing file, but I removed a few
inconsistencies rather than reimplementing them. :)
@mattmccutchen-cci

mattmccutchen-cci commented Mar 8, 2021

Copy link
Copy Markdown
Collaborator Author

The new run looks at least as good as the previous run, so I'm declaring this ready for review.

@mattmccutchen-cci mattmccutchen-cci marked this pull request as ready for review March 8, 2021 21:52
features I learned about while working on the clang/test/3C style.

(Namely, textwrap.dedent and f'{foo}' string interpolation.)

There is no change to the generated workflow file.
Comment thread .github/workflows/main.yml
- Change to use -output-dir as suggested by Mike.

- Correctly handle the benchmarks that have a separate build directory
  (libarchive and zlib) using the new convert_project option.

- Stop the workflow at the beginning if it is out of date with
  generate-workflow.py, since I almost made that mistake once.

- Add -Wno-enum-conversion compiler option to fix vsftpd.

@john-h-kastner john-h-kastner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Lets merge this so it's easier to test our fixes against it.

One suggestion. The python script could include a call to yamllint to sanity check the generated workflow. Easy enough to run manually, but it might catch trivial errors in future changes.

mattmccutchen-cci added a commit to correctcomputation/checkedc-clang that referenced this pull request Mar 15, 2021
- Add `--build_dir` option to `convert_project` to specify a build
  directory (containing `compile_commands.json`) different from the
  source directory (which is scanned for source files to update
  includes). This is needed by LibArchive and ZLib, which currently set
  `--project_path` to the build directory, which breaks include
  updating.

- Switch from `-output-postfix` to `-output-dir` to reduce the amount of
  code needed to move converted source files into place for the
  post-conversion build. Theoretically, this would break any caller that
  relies on the current location of the output files, but the current
  benchmark test workflow doesn't use the output files at all.

- To make the output directory location more predictable, always set the
  3C base directory equal to the `--project_path` specified to
  `convert_project` rather than the common ancestor directory of the
  source files in the compilation database. This will temporarily break
  LibArchive and ZLib even further until they are migrated to the new
  usage of the `--project_path` and `--build_dir` options.

- Add `--extra-3c-arg` to `convert_project` that can be used to pass the
  `-alltypes` flag through to 3C.

mwhicks1/3c-actions#7 will use the new
features to improve the benchmark test workflow.
@mattmccutchen-cci

Copy link
Copy Markdown
Collaborator Author

One suggestion. The python script could include a call to yamllint to sanity check the generated workflow. Easy enough to run manually, but it might catch trivial errors in future changes.

A good idea but I don't want to take the time to do it now, so I've filed #9 for it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants