Despite the user-supplied conduit token being passed during landing,
the local git config is still used when applying arc patch, which overwrites
the committer (but not author) information. This patch uses the conduit token
to fetch the user's name and email to use as committer, ensuring the land bot
is not taking credit as the committer (which happened for 04394f92dce and some others).
Details
- Reviewers
deadalnix - Group Reviewers
Restricted Project
CONDUIT_TOKEN=<token> ./land-patch.sh --dry-run --revision Dxxxx
Try changing out CONDUIT_TOKEN with some other user's token and observe with git show
that the committer information is set appropriately.
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- preserve-committer
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Event Timeline
Can you explain how it works ? Are the exported variables interpreted by arc land (I don't see any reference in the doc) ?
contrib/source-control-tools/land-patch.sh | ||
---|---|---|
120 | Is it guaranteed that the phabricator name/email pair is the same as the user git's name/email ? |
contrib/source-control-tools/land-patch.sh | ||
---|---|---|
120 | It is not. |
GIT_COMMITER_(NAME|EMAIL) are git environment variables that can be used to override committer info. This is relevant
during the arc patch call in autopatch.sh. Normally the committer info is the current user's config, but abc-bot is landing this change
so the committer needs to be set to match the info of the supplied conduit token.
contrib/source-control-tools/land-patch.sh | ||
---|---|---|
120 | The phab name/email is set via your profile. If this configuration turns out to be not acceptable, an alternative approach is to pass the user's configured name/email at the time of calling arc land-via-bot. I'm happy to hear use cases where you'd prefer name/email to be different between git and phab. |
The latest staging repo state may match the patch, but the title, summary, test plan, and reviewers are not guaranteed to be up-to-date.
contrib/source-control-tools/land-patch.sh | ||
---|---|---|
120 | Just have arc send the relevant infos, instead of trying to guess, possibly wrong. |