-
Notifications
You must be signed in to change notification settings - Fork 27
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
Axom Dev Guide Updates #1430
Axom Dev Guide Updates #1430
Conversation
<https://github.com/LLNL/axom/issues/new>`_ and note it in the ``known bugs`` | ||
section of the release notes. Alternatively, if time permits, fix the | ||
bug in a different branch and create a pull request as you would do during | ||
regular development. After the bus is resolved and that pull request is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
regular development. After the bus is resolved and that pull request is | |
regular development. After the bug is resolved and that pull request is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Fixed.
and configurations of software packages. It has recipes for building each package | ||
with available variants to customize it to your needs. For example, Axom | ||
has variants for Fortran, MPI, and others. The recipes drive | ||
the individual packages build systems and manage packages they depend on. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the individual packages build systems and manage packages they depend on. | |
the individual packages' build systems and manage packages they depend on. |
package, plural, possessive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Fixed.
Spack also handles system level packages, so you can describe where they are on your | ||
system instead of building them from scratch. You will need to describe which compilers | ||
system instead of building them from scratch. You will need to describe which compilers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What? Only single-space between sentences? : )
We try to minimize these, but we often have to alter the existing Spack packages to: apply fixes | ||
before pushing them up to Spack proper, or alter recipes in ways that are Axom specific. | ||
Such overrides do not happen at the Spack level, but at the next level, Uberenv | ||
described below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest:
- remove the colon from "to: apply fixes"
- remove the comma from "up to Spack proper, or alter recipes"
- either add a comma, making it "the next level, Uberenv, described below," or add parentheses, making it "the next level, Uberenv (described below)."
The last point is needed because "described below" is an adjective phrase modifying "Uberenv." Without the comma or the parens, overrides happen at the noun phrase "Uberenv described below" which makes less sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Change made.
copy of the Axom repository in which the script is run. After building all of TPL sets | ||
for a platform, it will test Axom against those TPL installs as well as the ``using-with-cmake`` | ||
example for correctness. This script stops at the first failed TPL build but |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy of the Axom repository in which the script is run. After building all of TPL sets | |
for a platform, it will test Axom against those TPL installs as well as the ``using-with-cmake`` | |
example for correctness. This script stops at the first failed TPL build but | |
copy of the Axom repository in which the script is run. After building all TPL sets | |
for a platform, it will attempt to build Axom and the ``using-with-cmake`` example against | |
each set of TPLs. This script stops at the first failed TPL build but |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommended change pushed.
to before it redirects the output from the command, i.e., ``[[log file: /path/to/log``. | ||
Due to the large amount of information printed to the screen during a full build, the build scripts | ||
redirect most build step output to log files. This output will tell you what command is being run, | ||
i.e., ``[EOE: some/command --with-options]``, and will tell you the log file being written |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EOE?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good lord. What was I doing?!?
Fixed.
|
||
#. Download the new release and override the source that is already there. | ||
This can often involve removing files no-longer needed but most of the | ||
current ones are a single header file. | ||
This may involve removing files no-longer needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may involve removing files no-longer needed. | |
This may involve removing files no longer needed. |
(minor suggestion for style and clarity)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
with a new set of TPLs. Typically, this process is followed when you want to | ||
update one or more TPLs which Axom depends on. After they are built and | ||
the required changes are merged into develop, they will be available for | ||
update one or more TPLs on which Axom depends. After they are built and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly:
update one or more TPLs on which Axom depends. After they are built and | |
update one or more TPLs. After they are built and |
They're our TPLs. We don't have to say again that we depend on them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, Rich! Much clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rhornung67 for this thorough update to our build and release process!
the end indicating which Axom build succeeded or failed. | ||
* ``build_src.py``: This script uses the existing host-configs in your local clone of the | ||
Axom repo, or a specific one you point at, and builds and tests Axom against them. It also | ||
tests the ``using-with-cmake`` example. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly helpful clarification:
tests the ``using-with-cmake`` example. | |
tests axom installation via the ``using-with-cmake`` example and axom tutorials. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Thank you.
.. important:: To install TPL builds to be shared by all Axom developers and used | ||
in our GitLab CI, you will need to become the Axom service user ``atk``. | ||
There is a clone of the Axom repo in the ``/usr/workspace/atk/axom_repo`` | ||
directory. After becoming ``atk``, you can go into that directory and | ||
switch to the branch you made to test your changes. Before running the | ||
TPL builds, make sure the branch is updated, including all submodules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention that this is only applicable to the subset of axom developers who can become atk
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's implied since the /usr/workspace/atk directory is not accessible if you do not xsu atk
and only members of the team can do that.
The main point I was trying to make is that you really need to become atk and use that clone of the repo to build and install the shared TPLs.
I've addressed all the reviewer comments so far. Thank you for the corrections and suggestions. If anyone else wants to review this, please do that soon. I will merge it when all checks are green. |
Summary
The RTD content for this PR is here: https://axom.readthedocs.io/en/task-rhornung67-devguide-tpls/docs/sphinx/dev_guide/index.html