Remove unused build options #1253
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This isn't sufficient to create the documentation as some of it is contained in MS Word files.
We don't distrubute tarballs anymore. That's handled by GitHub.
hainest
added
build
code cleanup
vsoch
reviewed
Apr 12, 2022
| @@ -163,78 +163,6 @@ endif() | |||
| set (VERSION_STRING "${DYNINST_MAJOR_VERSION}.${DYNINST_MINOR_VERSION}.${DYNINST_PATCH_VERSION}") | |||
| set (DYNINST_NAME "DyninstAPI-${VERSION_STRING}") | |||
|
|
|||
| if(BUILD_TARBALLS) | |||
There was a problem hiding this comment.
This giant big beautiful block of red deletion is very satisfying, +1 destroy it.
vsoch
reviewed
Apr 12, 2022
| option (BUILD_RTLIB_32 "Build 32-bit runtime library on mixed 32/64 systems" OFF) | ||
|
|
||
| option(BUILD_DOCS "Build manuals from LaTeX sources" ON) |
There was a problem hiding this comment.
Would be nice to someday have web docs because reading pdfs is sooooo 2009.
kupsch
reviewed
Apr 12, 2022
There was a problem hiding this comment.
Removing BUILD_TARBALLS is good as it is unused.
The BUILD_DOCS currently is broken; make docs sort of works the first time, but is broken for subsequent invocations. It would be nice to have a make target to build the latex docs that had proper dependencies so it would rebuild correctly. If this is removed, there needs to be instructions on how build the docs added to the repo and perhaps a script to create the pdfs.
|
@kupsch As I noted in the commit message, building the latex files is insufficient for creating the complete set of documentation PDFs because dyninstAPI and proccontrol use Word documents that have to be manually exported as PDFs. Instructions for building the documentation are part of the release process docs. As @vsoch noted, we need to move away from this form of delivering documentation. I would really like to host them on readthedocs so that we can actually have a one-button release process that synchronously updates everything. The biggest hurdle to that right now is converting latex to sphynx. That requires converting latex to restructuredtext which can be done with pandoc. It's just a matter of finding time to make sure it works. Also, automating documentation deployment means we can test the code examples to make sure they stay up-to-date. |
|
I'm pretty good at that, I could probably help in a week or so, have too many "after work personal projects" right now but probably that won't be the case after I finish some up! I've done readthedocs with rst a gazillion times. |
|
@vsoch I wasn't hinting- I promise. 😃 If you have the cycles and will, I certainly won't turn away the help, though. |
|
yeah I can add to my list of "stuff to do" I'll get to it sooner than later. :) |
|
@hainest I understand that the latex documentation is only part of the documentation, but it wasn't clear to me how someone that forked the repo would know how to build the latex documentation; it would require them to know the existence of the google doc. If we are moving from latex soon then this is OK and the merge can proceed. If not, then putting the instructions in README.md (or similar) that is versioned in the repo would be useful. |
|
@kupsch Oh, sorry, I didn't get that part of what you were saying. As far as I know, no one but us has ever tried to build the documentation. Even the Red Hat folks use the generated PDFs. I'm pretty sure the BUILD_DOCS was a holdover from the autotools->CMake conversion. I would really like to see us move to readthedocs soon. Creating the documentation PDFs is by far the biggest pain of doing a release. Since Vanessa offered a bit of her time, I think the two of us can put our heads together to do a bulk export. There are still doc bugs that need to be fixed (#800, #711, #394), but we can do that afterward. |
wcohen
pushed a commit
to wcohen/dyninst
that referenced
this pull request
Jul 26, 2022
* Remove BUILD_DOCS This isn't sufficient to create the documentation as some of it is contained in MS Word files. * Remove BUILD_TARBALLS We don't distrubute tarballs anymore. That's handled by GitHub.
hainest
added a commit
that referenced
this pull request
Dec 12, 2022
* Update dependency versions in base container config This should have been part of #1211 * Docker: adding a workflow for release (#1219) Currently we do the entire build from scratch for release, and this strategy was chosen for the cleanest build. However we can use the same strategy as we do for testing and take advantage of the base container. This PR adds a workflow to do that Signed-off-by: vsoch <vsoch@users.noreply.github.com> Co-authored-by: vsoch <vsoch@users.noreply.github.com> * Remove usage of DW_AT_MIPS_linkage_name (#1223) This isn't part of DWARF4 or DWARF5. Co-authored-by: Tim Haines <thaines@cs.wisc.edu> * Docker: testing workflow to run libabigail (#1220) Run libabigail's abidiff to detect ABI breakages between the current PR and the last successful build as well as the current PR and the last release. * testing workflow to run libabigail! I am not sure if the artifact is extracted relative or not, so will need to update the workflow to account for that. This is also a new strategy that will try to do the new build/retrieval of artifacts from previous containers - have not tried this yet! Signed-off-by: vsoch <vsoch@users.noreply.github.com> Co-authored-by: vsoch <vsoch@users.noreply.github.com> Co-authored-by: Tim Haines <thaines.astro@gmail.com> * Correctly propagate pc ranges for blocks and local variables (#1226) - fix valid pc address ranges for local variables that are declared in a sub-block (brace, try and catch blocks) of their function - fix Context class's constructors so they correctly initialize and copy all member (use in-class initialize and default constructors) * Fix warnings with cmake's MINSIZEREL build type (#1235) - Explicitly define optimization flags for cmake's MINSIZEREL build type. Cmake by default includes -DNDEBUG. This results in assert being defined to do nothing. Dyninst use asserts as a fatal error reporting mechanism and expects assert to not return if the condition is always false. If assert can return then many warnings are produced due to this unexpect path. - Remove defining NDEBUG for REL build types if using the MSC compiler * Remove BUILD_RT option (#1238) When not enabled, the dyninstAPI_RT/cmake_install.cmake isn't present. This has been broken since it was implemented by e1afc69 in 2016. * Add parsing of names for inlined functions in DWARF (#1237) * Clean up and enhance debug messages * Add parsing of names for inlined functions Co-authored-by: Tim Haines <thaines@cs.wisc.edu> * Remove void pointer arithmetic when using Valgrind annotations (#1236) gcc allows this as an extension by considering sizeof(void) to be 1 (http://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html). * Docker: use more OS packages for dependencies (#1221) * Docker: use more OS packages for dependencies This should have been part of #1211. * Docker: don't build Dyninst through spack for the environment (#1222) This results in two installations: one in /opt/dyninst-dev/.spack/include and one in /opt/dyninst-dev/install/dyninst. The former is not updated when testing a PR, so causes public header conflicts. * Docker: use external-tests instead of testsuite in base image (#1209) This is in preparation for using -Werror when building Dyninst in the Github workflow. The test suite does not currently build cleanly, so we need a different set of tests. * Docker: make compile warnings fatal (#1242) * Remove unused git files (#1244) .gitmodules hasn't been used since the test suite was moved to its own repository. .github_changelog_generate hasn't been used since v10.0.0 * Improve compiler diagnostic suppression handling (#1239) - Create compiler specific diagnostic suppression macros to suppress a type of warning for a region of code that are consistently defined based on the build environment. - Replace current multi-line preprocessor #if and #pragma statements with one macro. - Add suppression for warnings in instructionAPI/src/Register.C when using gcc 6-8. * Remove MSC compiler warning suppressions (#1239) Likely no longer needed with current MSC compiler and will hide problems if MSC is used. - Removed diagnostic suppressing pragmas - Removed compiler command line suppression options * Fix frame-larger-than warning (#1239) - Increase frame size max when using gcc 6 for non-debug builds and for all debug builds (needed for rhel's gcc) to compile instructionAPI/src/AMDGPU/cdna2/InstructionDecoder-amdgpu-cdna2.C * Add cmake option to disable diagnostic suppressions (#1239) - DYNINST_DISABLE_DIAGNOSTIC_SUPPRESSIONS option: if set, disable all warning suppressions and frame size limit overrides * Add compiler warning related cmake options (#1239) - DYNINST_WARNINGS_AS_ERRORS option: if set, treat warnings as errors - DYNINST_EXTRA_WARNINGS option: additional warning options to test for and use if valid with the current compiler * Remove unneeded #pragma's (#1240) - Remove once and interface pragmas - Replace use of "external/stdint-win.h" and "external/inttypes.h" with <stdint.h> and <inttypes.h> * remove unused files containing pragmas (#1240) * Cleanup (remove) ancient linux kernel support (#1241) - Remove check and message for ancient linux kernel - Remove support for old mechanism to find process's tasks * Use bfd linker for LTO (#1248) The gold linker crashes and is no longer supported. * Add cmake options for C/C++ language standards (#1246) Add cmake options to set C/C++ language standard versions used to build to facilitate testing. - DYNINST_C_LANGUAGE_STANDARD cmake option: C Standard version - DYNINST_CXX_LANGUAGE_STANDARD cmake option: C++ Standard version * Make dyninstAPI_RT files build with standard C (#1246) - use same language standards as the rest of Dyninst - define _DEFAULT_SOURCE in some dyninstAPI_RT source files so functions and macros used are defined when using standard C * Fix format string errors in stackwalk/callchecker.C (#1250) These are only present when SW_ANALYSIS_STEPPER=OFF. * Redo finalization to get correct function boundiaries when (#1249) there are many tail call correction * Fix dyninstAPI_RT files to build with older glibc (#1252) Replace feature test macro _DEFAULT_SOURCE with _GNU_SOURCE to support older version of glibc * Remove unused build options (#1253) * Remove BUILD_DOCS This isn't sufficient to create the documentation as some of it is contained in MS Word files. * Remove BUILD_TARBALLS We don't distrubute tarballs anymore. That's handled by GitHub. * start of work to refactor the docs to use readthedocs this is the first pass to format the previous latex into rst. I have done the conversion and only started to go through ensuring that content is preserved (meaning I did not miss anything) and all the code blocks are formatted (after the automated conversion they indeed are not!) I will want a few more hours to finish this up, and then we need to discuss deployment. E.g., I recommend readthedocs so you can automate deployment and keep versioned docs. We can also deploy to github pages (I can make a workflow) but I do not have a good suggestion for versioning things that way. I also have not added in an ability to still render the pdfs if that is desired, which I think should be possibly. Finally, it would be nice if some of these docs could render from docstrings - I know how to do this for Python so I wonder if Cpp is that much different. It is probably terrible because it is cpp, but what can you do? Signed-off-by: vsoch <vsoch@users.noreply.github.com> * adding missing figures and re-creating tables in format that will render Signed-off-by: vsoch <vsoch@users.noreply.github.com> * use different version of checkout to get around checkout bug Signed-off-by: vsoch <vsoch@users.noreply.github.com> * try fix for gha Signed-off-by: vsoch <vsoch@users.noreply.github.com> Signed-off-by: vsoch <vsoch@users.noreply.github.com> Co-authored-by: Tim Haines <thaines.astro@gmail.com> Co-authored-by: vsoch <vsoch@users.noreply.github.com> Co-authored-by: Tim Haines <thaines@cs.wisc.edu> Co-authored-by: kupsch <kupsch@cs.wisc.edu> Co-authored-by: Xiaozhu Meng <mxz297@gmail.com>
hainest
added a commit
that referenced
this pull request
Dec 13, 2022
* Update dependency versions in base container config This should have been part of #1211 * Docker: adding a workflow for release (#1219) Currently we do the entire build from scratch for release, and this strategy was chosen for the cleanest build. However we can use the same strategy as we do for testing and take advantage of the base container. This PR adds a workflow to do that Signed-off-by: vsoch <vsoch@users.noreply.github.com> Co-authored-by: vsoch <vsoch@users.noreply.github.com> * Remove usage of DW_AT_MIPS_linkage_name (#1223) This isn't part of DWARF4 or DWARF5. Co-authored-by: Tim Haines <thaines@cs.wisc.edu> * Docker: testing workflow to run libabigail (#1220) Run libabigail's abidiff to detect ABI breakages between the current PR and the last successful build as well as the current PR and the last release. * testing workflow to run libabigail! I am not sure if the artifact is extracted relative or not, so will need to update the workflow to account for that. This is also a new strategy that will try to do the new build/retrieval of artifacts from previous containers - have not tried this yet! Signed-off-by: vsoch <vsoch@users.noreply.github.com> Co-authored-by: vsoch <vsoch@users.noreply.github.com> Co-authored-by: Tim Haines <thaines.astro@gmail.com> * Correctly propagate pc ranges for blocks and local variables (#1226) - fix valid pc address ranges for local variables that are declared in a sub-block (brace, try and catch blocks) of their function - fix Context class's constructors so they correctly initialize and copy all member (use in-class initialize and default constructors) * Fix warnings with cmake's MINSIZEREL build type (#1235) - Explicitly define optimization flags for cmake's MINSIZEREL build type. Cmake by default includes -DNDEBUG. This results in assert being defined to do nothing. Dyninst use asserts as a fatal error reporting mechanism and expects assert to not return if the condition is always false. If assert can return then many warnings are produced due to this unexpect path. - Remove defining NDEBUG for REL build types if using the MSC compiler * Remove BUILD_RT option (#1238) When not enabled, the dyninstAPI_RT/cmake_install.cmake isn't present. This has been broken since it was implemented by e1afc69 in 2016. * Add parsing of names for inlined functions in DWARF (#1237) * Clean up and enhance debug messages * Add parsing of names for inlined functions Co-authored-by: Tim Haines <thaines@cs.wisc.edu> * Remove void pointer arithmetic when using Valgrind annotations (#1236) gcc allows this as an extension by considering sizeof(void) to be 1 (http://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html). * Docker: use more OS packages for dependencies (#1221) * Docker: use more OS packages for dependencies This should have been part of #1211. * Docker: don't build Dyninst through spack for the environment (#1222) This results in two installations: one in /opt/dyninst-dev/.spack/include and one in /opt/dyninst-dev/install/dyninst. The former is not updated when testing a PR, so causes public header conflicts. * Docker: use external-tests instead of testsuite in base image (#1209) This is in preparation for using -Werror when building Dyninst in the Github workflow. The test suite does not currently build cleanly, so we need a different set of tests. * Docker: make compile warnings fatal (#1242) * Remove unused git files (#1244) .gitmodules hasn't been used since the test suite was moved to its own repository. .github_changelog_generate hasn't been used since v10.0.0 * Improve compiler diagnostic suppression handling (#1239) - Create compiler specific diagnostic suppression macros to suppress a type of warning for a region of code that are consistently defined based on the build environment. - Replace current multi-line preprocessor #if and #pragma statements with one macro. - Add suppression for warnings in instructionAPI/src/Register.C when using gcc 6-8. * Remove MSC compiler warning suppressions (#1239) Likely no longer needed with current MSC compiler and will hide problems if MSC is used. - Removed diagnostic suppressing pragmas - Removed compiler command line suppression options * Fix frame-larger-than warning (#1239) - Increase frame size max when using gcc 6 for non-debug builds and for all debug builds (needed for rhel's gcc) to compile instructionAPI/src/AMDGPU/cdna2/InstructionDecoder-amdgpu-cdna2.C * Add cmake option to disable diagnostic suppressions (#1239) - DYNINST_DISABLE_DIAGNOSTIC_SUPPRESSIONS option: if set, disable all warning suppressions and frame size limit overrides * Add compiler warning related cmake options (#1239) - DYNINST_WARNINGS_AS_ERRORS option: if set, treat warnings as errors - DYNINST_EXTRA_WARNINGS option: additional warning options to test for and use if valid with the current compiler * Remove unneeded #pragma's (#1240) - Remove once and interface pragmas - Replace use of "external/stdint-win.h" and "external/inttypes.h" with <stdint.h> and <inttypes.h> * remove unused files containing pragmas (#1240) * Cleanup (remove) ancient linux kernel support (#1241) - Remove check and message for ancient linux kernel - Remove support for old mechanism to find process's tasks * Use bfd linker for LTO (#1248) The gold linker crashes and is no longer supported. * Add cmake options for C/C++ language standards (#1246) Add cmake options to set C/C++ language standard versions used to build to facilitate testing. - DYNINST_C_LANGUAGE_STANDARD cmake option: C Standard version - DYNINST_CXX_LANGUAGE_STANDARD cmake option: C++ Standard version * Make dyninstAPI_RT files build with standard C (#1246) - use same language standards as the rest of Dyninst - define _DEFAULT_SOURCE in some dyninstAPI_RT source files so functions and macros used are defined when using standard C * Fix format string errors in stackwalk/callchecker.C (#1250) These are only present when SW_ANALYSIS_STEPPER=OFF. * Redo finalization to get correct function boundiaries when (#1249) there are many tail call correction * Fix dyninstAPI_RT files to build with older glibc (#1252) Replace feature test macro _DEFAULT_SOURCE with _GNU_SOURCE to support older version of glibc * Remove unused build options (#1253) * Remove BUILD_DOCS This isn't sufficient to create the documentation as some of it is contained in MS Word files. * Remove BUILD_TARBALLS We don't distrubute tarballs anymore. That's handled by GitHub. * start of work to refactor the docs to use readthedocs this is the first pass to format the previous latex into rst. I have done the conversion and only started to go through ensuring that content is preserved (meaning I did not miss anything) and all the code blocks are formatted (after the automated conversion they indeed are not!) I will want a few more hours to finish this up, and then we need to discuss deployment. E.g., I recommend readthedocs so you can automate deployment and keep versioned docs. We can also deploy to github pages (I can make a workflow) but I do not have a good suggestion for versioning things that way. I also have not added in an ability to still render the pdfs if that is desired, which I think should be possibly. Finally, it would be nice if some of these docs could render from docstrings - I know how to do this for Python so I wonder if Cpp is that much different. It is probably terrible because it is cpp, but what can you do? Signed-off-by: vsoch <vsoch@users.noreply.github.com> * adding missing figures and re-creating tables in format that will render Signed-off-by: vsoch <vsoch@users.noreply.github.com> * use different version of checkout to get around checkout bug Signed-off-by: vsoch <vsoch@users.noreply.github.com> * try fix for gha Signed-off-by: vsoch <vsoch@users.noreply.github.com> Signed-off-by: vsoch <vsoch@users.noreply.github.com> Co-authored-by: Tim Haines <thaines.astro@gmail.com> Co-authored-by: vsoch <vsoch@users.noreply.github.com> Co-authored-by: Tim Haines <thaines@cs.wisc.edu> Co-authored-by: kupsch <kupsch@cs.wisc.edu> Co-authored-by: Xiaozhu Meng <mxz297@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We should revisit using CPack.