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

Add git safe dir override for Kokkos docker builds #452

Merged
merged 5 commits into from
Jun 21, 2023

Conversation

mlxd
Copy link
Member

@mlxd mlxd commented May 23, 2023

Before submitting

Please complete the following checklist when submitting a PR:

  • All new features must include a unit test.
    If you've fixed a bug or added code that should be tested, add a test to the
    tests directory!

  • All new functions and code must be clearly commented and documented.
    If you do make documentation changes, make sure that the docs build and
    render correctly by running make docs.

  • Ensure that the test suite passes, by running make test.

  • Add a new entry to the .github/CHANGELOG.md file, summarizing the
    change, and including a link back to the PR.

  • Ensure that code is properly formatted by running make format.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.


Context: To avoid issues around git directory safety and ownership as seen in https://github.com/PennyLaneAI/pennylane-lightning/actions/runs/5058417626/jobs/9078527537#step:6:153 we must explicitly mark the mounted /io directory as safe to ensure cross-platforms builds continue to work.

Description of the Change: Marked /io directory as safe for docker-enabled cross-platform builds on arm and powerpc.

Benefits: Ensure aarch64 and ppc64le wheels will successfully continue to build on release.

Possible Drawbacks:

Related GitHub Issues:

@mlxd mlxd marked this pull request as ready for review May 23, 2023 15:02
@codecov
Copy link

codecov bot commented May 23, 2023

Codecov Report

Merging #452 (11b86c4) into v0.31.0_rc (5b5fd0c) will increase coverage by 7.75%.
The diff coverage is 100.00%.

@@              Coverage Diff               @@
##           v0.31.0_rc     #452      +/-   ##
==============================================
+ Coverage       92.07%   99.82%   +7.75%     
==============================================
  Files              50       50              
  Lines            4667     4667              
==============================================
+ Hits             4297     4659     +362     
+ Misses            370        8     -362     
Impacted Files Coverage Δ
pennylane_lightning/_version.py 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

@mlxd mlxd requested a review from a team May 23, 2023 16:02
Copy link
Contributor

@AmintorDusko AmintorDusko left a comment

Choose a reason for hiding this comment

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

Hi @mlxd, do you happen to have any insight on what might have caused this error message to appear?

@AmintorDusko AmintorDusko requested a review from a team May 23, 2023 17:02
@mlxd
Copy link
Member Author

mlxd commented May 23, 2023

Hi @mlxd, do you happen to have any insight on what might have caused this error message to appear?

Without explicitly going through the history of invalidated Kokkos caches and the associated Docker images used in the builds, I cannot tell. Though, I suspect it is due to a change in either git versions, or how docker handles permissions across mounts.

Copy link
Contributor

@AmintorDusko AmintorDusko left a comment

Choose a reason for hiding this comment

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

Hi @mlxd, do you happen to have any insight on what might have caused this error message to appear?

Without explicitly going through the history of invalidated Kokkos caches and the associated Docker images used in the builds, I cannot tell. Though, I suspect it is due to a change in either git versions, or how docker handles permissions across mounts.

That makes sense. We are good to go!

@mlxd mlxd requested a review from vincentmr May 24, 2023 16:21
@vincentmr vincentmr changed the base branch from master to v0.31.0_rc June 21, 2023 14:02
Copy link
Contributor

@vincentmr vincentmr left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks @mlxd

@mlxd mlxd merged commit b8117f6 into v0.31.0_rc Jun 21, 2023
@mlxd mlxd deleted the qemu_git_ownership branch June 21, 2023 14:29
mlxd added a commit that referenced this pull request Jun 26, 2023
* Initial commit for Lightning v0.31.0_rc

* Update changelog.

* Bump pennylane_requires to 0.30.

* Add git safe dir override for Kokkos docker builds (#452)

* Add git safe dir override for Kokkos docker builds

* Auto update version

* Update changelog

---------

Co-authored-by: Dev version update bot <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Vincent Michaud-Rioux <[email protected]>

* Specify PYTHON_EXECUTABLE in CMAKE_ARGS.

* Fix case of Python_EXECUTABLE CMAKE var.

* Put back Python_EXECUTABLE in setup.py. (#461)

* Put back Python_EXECUTABLE in setup.py.

* Auto update version

* Trigger CI

* Fix PYTHON_EXECUTABLE case.

* Fix PYTHON_EXECUTABLE case MacOS.

* Update changelog [skip ci].

* Trigger CI

---------

Co-authored-by: Dev version update bot <github-actions[bot]@users.noreply.github.com>

* Build against PL-rc branch.

* --force-reinstall autograd numpy==1.23.5

* Revert changes.

* Update changelog.

* Revert changes made on wrong branch.

* Trigger CI

* Auto update version

* Revert RC test branches to master

---------

Co-authored-by: Vincent Michaud-Rioux <[email protected]>
Co-authored-by: Dev version update bot <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Vincent Michaud-Rioux <[email protected]>
Co-authored-by: Amintor Dusko <[email protected]>
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