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.

Sign up for GitHub

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

Update contributing and repo management docs for github first #2339

Merged
merged 2 commits into from
Jun 27, 2020
Merged

Update contributing and repo management docs for github first #2339

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08ba29a
Update contributing and repo management docs for github first
GMNGeoffrey Jun 27, 2020
a6e7834
Fix typos
GMNGeoffrey Jun 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ just a few small guidelines you need to follow.
## Contributor License Agreement

Contributions to this project must be accompanied by a Contributor License
Agreement. You (or your employer) retain the copyright to your contribution;
Agreement. You (or your employer) retain the copyright to your contribution;
GMNGeoffrey marked this conversation as resolved.
Show resolved Hide resolved
this simply gives us permission to use and redistribute your contributions as
part of the project. Head over to <https://cla.developers.google.com/> to see
your current agreements on file or to sign a new one.
Expand All @@ -24,8 +24,10 @@ Changes only tweaking style are unlikely to be accepted unless they are applied
consistently across the project. Most of the code style is derived from the
[Google Style Guides](http://google.github.io/styleguide/) for the appropriate
language and is generally not something we accept changes on (as clang-format
and clang-tidy handle that for us). Improvements to code structure and clarity
are welcome but please file issues to track such work first.
and clang-tidy handle that for us). The compiler portion of the project follows
[MLIR style](https://mlir.llvm.org/getting_started/DeveloperGuide/#style-guide).
Improvements to code structure and clarity are welcome but please file issues to
track such work first.

## Code reviews

Expand All @@ -39,25 +41,26 @@ information on using pull requests.
Several of our presubmit builds will only run automatically if you are a project
collaborator. Otherwise a collaborator must label the PR with "kokoro:run". If
you are sending code changes to the project, please ask to be added as a
collaborator, so that these can run automatically.
collaborator, so that these can run automatically. It is generally expected that
PRs will only be merged when all presubmit checks are passing. In some cases,
pre-existing failures may be ignored.

## Peculiarities
## Merging

We use a GitHub integration to import PRs into our upstream (Google internal)
source code management. Once it is approved internally, each PR will be merged
into the master branch as a single commit by the same tooling. The description
will match the PR title followed by the PR description. Accordingly, please
write these as you would a helpful commit message. Please also keep PRs small
(focused on a single issue) to streamline review and ease later culprit-finding.
After review and presubmit checks, PRs should be merged with a "squash and
merge". The squashed commit summary should match the PR title and the commit
description should match the PR body. Accordingly, please write these as you
would a helpful commit message. Please also keep PRs small (focused on a single
issue) to streamline review and ease later culprit-finding. It is assumed that
the PR author will authors merge their change unless they ask someone else to
merge it for them (e.g. because they don't have write access).

As part of a migration to make the project GitHub-first, our default branch is
currently called `google` and all PRs should be directed there. This is an
intermediate state. See
https://groups.google.com/d/msg/iree-discuss/F07vsG9Ah4o/uAIusKO-BQAJ
## Peculiarities

Our documentation on
[repository management](https://github.com/google/iree/blob/master/docs/repository_management.md)
has more information on some of the oddities in our repository setup and
workflows. For the most part, these should be transparent to normal developer
workflows.

## Community Guidelines
Expand Down
64 changes: 21 additions & 43 deletions docs/repository_management.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ external contributors, but they are documented here for clarity and
transparency. If any of these things are particularly troublesome or painful for
your workflow, please reach out to us so we can prioritize a fix.

NOTE: We are currently in the process of migrating our repository to be
GitHub-first and hide the merging complexity in a separate `google` feature
branch so that standard development workflows don't have to bear the cost for
every contribution. During this part of the migration period, please direct PRs
to the `google` branch (which will be marked as the default branch). See
https://groups.google.com/d/msg/iree-discuss/F07vsG9Ah4o/uAIusKO-BQAJ.
## Branches

The default branch is called `main`. PRs should be sent there. We also have a
`google` branch that is synced from the Google internal source repository. This
branch is merged from and to `main` frequently to usptream any changes coming
GMNGeoffrey marked this conversation as resolved.
Show resolved Hide resolved
from `google` and as part of updating the internal source repository. For the
most part this integration with `google` should have minimal effect on normal
developer workflows.

## Dependencies

Expand Down Expand Up @@ -46,6 +48,11 @@ Shortcut commands (read below for full documentation):

### The special relationship with LLVM and TensorFlow

TL;DR: Dependency management with TensorFlow and LLVM are managed as part of the
integration with Google's internal source repository and updates will be picked
up by the `main` branch at an approximately daily cadence when merging in the
`google` branch.

Currently, the two most challenging projects to manage as dependencies are
TensorFlow and LLVM. Both are typically pinned to specific versions that are
integrated in the Google source repository at a cadence up to many times per
Expand All @@ -67,15 +74,14 @@ upstream, we will need to integrate this newer commit in the internal Google
source of truth prior to accepting code that depends on it.

LLVM is integrated into Google's source repository at a cadence up to several
times a day. When this necessitates updating IREE to changing LLVM and MLIR
APIs, these updates are performed atomically as part of the integration.
Previously this did not trigger a change to the LLVM submodule version in open
source, which would result in breakages due to version skew. We are
transitioning to make each integration of LLVM update IREE's submodule in OSS.
This means that API updates will have some chance of not breaking the build.
However, it can introduce breakages as well, especially in the Bazel build where
we still have to wait to update the BUILD files where necessary. We are
continuing to work on improving this situation.
times a day. The LLVM submodule version used by IREE is also updated as part of
this integration. When this necessitates updating IREE to changing LLVM and MLIR
APIs, these updates are performed atomically as part of the integration. However
the changes performed here only guarantee that the internal build is not broken,
which means that such integrations may very well break the OSS build on the
`google` branch (in particular, it does not update teh LLVM BUILD files). The
IREE build cop will fix the breakage on the `google` branch. We are continuing
to work on improving this situation.

### Tasks:

Expand Down Expand Up @@ -147,31 +153,3 @@ as usual.
Any CI systems or other things that retrieve an arbitrary commit should invoke
this prior to running just to make sure that their git view of the submodule
state is consistent.

#### Updating TensorFlow and LLVM versions

WARNING: These scripts have not been updated to reflect the new tooling to
automatically update the LLVM hash directly from Google's source repository
rather than through TensorFlow.

```shell
# By default, updates third_party/tensorflow to the remote head and
# third_party/llvm-project to the commit that tensorflow references.
# Also updates the IREE mirrors of key build files. All changes will
# be staged for commit.
./scripts/git/update_tf_llvm_submodules.py

# You can also select various other options:
# Don't update the tensorflow commit, just bring LLVM to the correct
# linked commit.
./scripts/git/update_tf_llvm_submodules.py --tensorflow_commit=KEEP

# Don't update the LLVM commit.
./scripts/git/update_tf_llvm_submodules.py --llvm_commit=KEEP

# Update to a specific TensorFlow commit (and corresponding LLVM).
./scripts/git/update_tf_llvm_submodules.py --tensorflow_commit=<somehash>

# Update to a specific LLVM commit.
./scripts/git/update_tf_llvm_submodules.py --llvm_commit=<somehash>
```