Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docker: use more OS packages for dependencies #1221

Merged
merged 3 commits into from
Mar 30, 2022

Conversation

hainest
Copy link
Contributor

@hainest hainest commented Mar 11, 2022

This should have been part of #1211

@hainest hainest added enhancement CI Related to continuous integration testing labels Mar 11, 2022
@hainest hainest requested a review from vsoch March 11, 2022 18:10
@hainest hainest self-assigned this Mar 11, 2022
@vsoch
Copy link
Contributor

vsoch commented Mar 11, 2022

Does slack external find it work to find those?

@hainest
Copy link
Contributor Author

hainest commented Mar 11, 2022

Does slack external find it work to find those?

No. It won't find libraries- just executables in $PATH. :(

@vsoch
Copy link
Contributor

vsoch commented Mar 11, 2022

Any reason to printf into the file vs. storing a packages.yaml alongside the container?

@hainest
Copy link
Contributor Author

hainest commented Mar 11, 2022

None other than I didn't know you could do that. If we have both a hand-written packages.yaml and are using spack external find, where would the best place be to keep the packages.yaml?

@vsoch
Copy link
Contributor

vsoch commented Mar 11, 2022

If you are just building the container, you'd keep it in the docker/ folder with COPY to where it needs to be and add it before you setup the environment.

@hainest hainest force-pushed the thaines/docker_more_os_packages branch from 5f62389 to 7b6c94d Compare March 30, 2022 15:04
@hainest
Copy link
Contributor Author

hainest commented Mar 30, 2022

@vsoch I was able to get two of the three added with spack find external. The last one I'll leave for now. You, @kupsch, @bigtrak, and I should have a chat (preferably in the soon-ish timeframe) to expand what we have now to support multiple OSes and what we want to do about version evolution for dependencies. I'd also like to think about abstracting the base containers into their own layers, if that makes sense for whatever we decide on an overarching architecture.

@vsoch
Copy link
Contributor

vsoch commented Mar 30, 2022

@hainest could you temporarily add the PR trigger to the workflow to build the new container (and double check it won't deploy) so we can be 100% sure it builds ok? once that is done ping me to take a look, then we can undo that change and merge.

@hainest
Copy link
Contributor Author

hainest commented Mar 30, 2022

Did I do that correctly?

@@ -14,6 +14,10 @@ on:
push:
branches:
- master

pull_request:
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep! If you are lazy in the future you can just do:

pull_request: []

@vsoch
Copy link
Contributor

vsoch commented Mar 30, 2022

@hainest looks good! Just remove the PR trigger and +1 to merge.

52 minutes - Dyninst you CHONK!

@hainest
Copy link
Contributor Author

hainest commented Mar 30, 2022

52 minutes?! That's a paddlin'.

@hainest hainest merged commit ba15a85 into master Mar 30, 2022
@hainest hainest deleted the thaines/docker_more_os_packages branch March 30, 2022 22:19
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
Labels
CI Related to continuous integration testing enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants