fb_backend: fix warning
ClosedPublic

Authored by nerdopolist on Dec 30 2017, 4:18 PM.

Details

Reviewers
bshah
Group Reviewers
KWin
Commits
R108:3c745bb5716c: fb_backend: fix warning
Summary

The warning that the format is unknown was being logged unconditionally

Diff Detail

Repository
R108 KWin
Branch
arcpatch-D9567
Lint
No Linters Available
Unit
No Unit Test Coverage
nerdopolist created this revision.Dec 30 2017, 4:18 PM
Restricted Application added subscribers: KWin, kwin. · View Herald TranscriptDec 30 2017, 4:18 PM
nerdopolist requested review of this revision.Dec 30 2017, 4:18 PM
bshah accepted this revision.Jan 6 2018, 4:03 PM
This revision is now accepted and ready to land.Jan 6 2018, 4:03 PM
cfeck added a subscriber: cfeck.Mar 15 2018, 1:13 AM

Could someone commit this? The author has no commit rights. If possible, please also check his other requests; see bug 387486.

I'm unable to download, arc patch fails with "Base commit is not in local repository"

I am able to download raw diff... ...did I upload something wrong?

graesslin added a comment.EditedMar 18 2018, 7:41 AM

I am able to download raw diff... ...did I upload something wrong?

I can download the raw diff as well, but that one is lacking the commit message and the authorship. This I cannot push it.

Did you upload a patch or a git patch?

rkflx added a subscriber: rkflx.EditedMar 18 2018, 7:54 AM

I'm unable to download, arc patch fails with "Base commit is not in local repository"

arc patch works fine for me. "Base commit is not in local repository" is just informational BTW, you also get this if it will work.

Are you sure you are in the correct repository, i.e. kwin? (Sorry for the dumb question ;)

In D9567#228364, @rkflx wrote:

I am able to download raw diff... ...did I upload something wrong?

arc patch works fine for me. "Base commit is not in local repository" is just informational BTW, you also get this if it will work.

For me it doesn't. It then tries to connect to github and asks for my github username and password. Which I don't have.

rkflx added a comment.EditedMar 18 2018, 8:20 AM

Sorry for intruding here Martin, but smoothing this out and understanding where we still have problems is important for T7116: Streamlined onboarding of new contributors ;)

I am able to download raw diff... ...did I upload something wrong?

I can download the raw diff as well, but that one is lacking the commit message and the authorship. This I cannot push it.

Did you upload a patch or a git patch?

Hovering over the timestamp, it says "Via Web". Until T5242 is solved, Phabricator will not take over any authorship information from patches uploaded via the web uploader. You can do one of two things now:

  • Ask the author to use Arcanist.
  • Ask the author to provide name and email, then issue git commit --amend --no-edit --author "name <name@domain>".

In D9567#228364, @rkflx wrote:

arc patch works fine for me. "Base commit is not in local repository" is just informational BTW, you also get this if it will work.

For me it doesn't. It then tries to connect to github and asks for my github username and password. Which I don't have.

This can be debugged further with arc --trace patch. My working theory is that your working directory is not a checkout of origin/master, but instead a branch with the upstream tracking branch set to GitHub perhaps (or some .gitconfig issues?). Note that arc patch basically does a git fetch.

If Arcanist is not used to upload the patch, arc patch won't be able to attach the patch to the original base commit, it will always use HEAD of the current branch.

Further option to try: --skip-dependencies

In D9567#228374, @rkflx wrote:

Sorry for intruding here Martin, but smoothing this out and understanding where we still have problems is important for T7116: Streamlined onboarding of new contributors ;)

All right, let's track that down.

This can be debugged further with arc --trace patch. My working theory is that your working directory is not a checkout of origin/master, but instead a branch with the upstream tracking branch set to GitHub perhaps (or some .gitconfig issues?). Note that arc patch basically does a git fetch.

 ARGV  '/usr/share/arcanist/bin/../scripts/arcanist.php' '--trace' 'patch' 'D9567'
 LOAD  Loaded "phutil" from "/usr/share/libphutil/src".
 LOAD  Loaded "arcanist" from "/usr/share/arcanist/src".
Config: Reading user configuration file "/home/martin/.arcrc"...
Config: Did not find system configuration at "/etc/arcconfig".
Working Copy: Reading .arcconfig from "/home/martin/src/kf5/kde/workspace/kwin/.arcconfig".
Working Copy: Path "/home/martin/src/kf5/kde/workspace/kwin" is part of `git` working copy "/home/martin/src/kf5/kde/workspace/kwin".
Working Copy: Project root is at "/home/martin/src/kf5/kde/workspace/kwin".
Config: Did not find local configuration at "/home/martin/src/kf5/kde/workspace/kwin/.git/arc/config".
>>> [0] <conduit> differential.querydiffs() <bytes = 75>
>>> [1] <http> https://phabricator.kde.org/api/differential.querydiffs
<<< [1] <http> 124,701 us
<<< [0] <conduit> 125,535 us
>>> [2] <conduit> user.whoami() <bytes = 117>
>>> [3] <http> https://phabricator.kde.org/api/user.whoami
<<< [3] <http> 55,301 us
<<< [2] <conduit> 55,423 us
>>> [4] <conduit> differential.querydiffs() <bytes = 156>
>>> [5] <http> https://phabricator.kde.org/api/differential.querydiffs
<<< [5] <http> 93,450 us
<<< [4] <conduit> 93,567 us
>>> [6] <exec> $ git symbolic-ref --quiet HEAD
<<< [6] <exec> 2,453 us
>>> [7] <exec> $ git rev-parse --symbolic-full-name 'master'@{upstream}
<<< [7] <exec> 2,013 us
>>> [8] <exec> $ git --version
<<< [8] <exec> 1,769 us
>>> [9] <exec> $ git ls-remote --get-url 'origin'
<<< [9] <exec> 1,925 us
>>> [10] <conduit> repository.query() <bytes = 194>
>>> [11] <http> https://phabricator.kde.org/api/repository.query
<<< [11] <http> 54,063 us
<<< [10] <conduit> 54,251 us
>>> [12] <exec> $ git diff --no-ext-diff --no-textconv --submodule=short --raw 'HEAD' --
>>> [13] <exec> $ git ls-files --others --exclude-standard
<<< [13] <exec> 4,764 us
<<< [12] <exec> 6,912 us
>>> [14] <exec> $ git diff-files --name-only
<<< [14] <exec> 3,295 us
>>> [15] <exec> $ git show -s --format='%H' '' --
<<< [15] <exec> 1,985 us
 INFO  Base commit is not in local repository; trying to fetch.
>>> [16] <exec> $ git fetch --quiet --all
Username for 'https://github.com': ^C

And my .git/config:

[core]
	repositoryformatversion = 0
	filemode = true
	bare = false
	logallrefupdates = true
[remote "origin"]
	url = kde:kwin
	fetch = +refs/heads/*:refs/remotes/origin/*
[remote "personal"]
	url = kde:clones/kwin/graesslin/kwin
	fetch = +refs/heads/*:refs/remotes/personal/*
[branch "master"]
	remote = origin
	merge = refs/heads/master
[gui]
	wmstate = normal
	geometry = 872x391+548+237 205 177
[branch "Plasma/5.0"]
	remote = origin
	merge = refs/heads/Plasma/5.0
[branch "Plasma/5.2"]
	remote = origin
	merge = refs/heads/Plasma/5.2
[branch "Plasma/5.3"]
	remote = origin
	merge = refs/heads/Plasma/5.3
[branch "Plasma/5.4"]
	remote = origin
	merge = refs/heads/Plasma/5.4
[branch "Plasma/5.6"]
	remote = origin
	merge = refs/heads/Plasma/5.6
[remote "subdiff"]
	url = https://github.com/subdiff/kwin.git
	fetch = +refs/heads/*:refs/remotes/subdiff/*
[branch "wip/planesupport"]
	remote = subdiff
	merge = refs/heads/wip/planesupport
[branch "Plasma/5.7"]
	remote = origin
	merge = refs/heads/Plasma/5.7
[remote "romangilg"]
	url = kde:clones/kwin/romangilg/kwin.git
	fetch = +refs/heads/*:refs/remotes/romangilg/*
[branch "romangilg/atomicmodesupport"]
	remote = romangilg
	merge = refs/heads/romangilg/atomicmodesupport
[branch "Plasma/5.8"]
	remote = origin
	merge = refs/heads/Plasma/5.8
[branch "Plasma/5.10"]
	remote = origin
	merge = refs/heads/Plasma/5.10
[branch "Plasma/5.11"]
	remote = origin
	merge = refs/heads/Plasma/5.11
[branch "Plasma/5.12"]
	remote = origin
	merge = refs/heads/Plasma/5.12

I now removed the github remotes and afterwards it worked.

rkflx added a comment.EditedMar 18 2018, 10:12 AM

@nerdopolist As you've been around for a while, please make the effort to set up Arcanist. Then do arc patch D9567 and re-upload with arc diff. Otherwise Martin does not have your contact information for landing the patch.


@graesslin Thanks for debugging…

Seems like --all of git fetch is the reason. Thinking about it, that's probably wanted behaviour, because in general the review workflow allows the author to choose arbitrary branches to base his work on, so Arcanist has to fetch all of them.

More specifically, the root cause is that Roman deleted his kwin fork from GitHub, so trying to access that fails, and git falls back to asking for credentials. If I change subdiff to kde (which is accessible), it works. I wonder why you never ran into this with arc patch before, as this is not really specific to this patch, but any invocation of arc patch should trigger it.

Conclusion: Our current strategy of pushing new contributors to use Arcanist is likely still the way to go. Your issue is something to be aware of, but I'm hesitant to clutter the docs with this very special case (will reconsider if this comes up more often, of course).

Happy hacking.

In D9567#228429, @rkflx wrote:
I wonder why you never ran into this with `arc patch` before, as this is not really specific to this patch, but any invocation of `arc patch` should trigger it.

I have an explanation for that: I have two systems I use for hacking and maybe the other one doesn't have that remote and by pure chance I always used that one.

In D9567#228429, @rkflx wrote:

@nerdopolist As you've been around for a while, please make the effort to set up Arcanist. Then do arc patch D9567 and re-upload with arc diff. Otherwise Martin does not have your contact information for landing the patch.


I'll see what I can try with Arcanist... ...but I could not figure out how to do the patch series with it...

nerdopolist edited the summary of this revision. (Show Details)
nerdopolist edited the test plan for this revision. (Show Details)

Try to make compatible with arc

Bring back the old diff

Make compatible with arc

Let me know if I did this correctly. I might need help with the other ones.
Trying to do arc patch D9556
and arc diff prompts for 3 others...

Let me know if I did this correctly. I might need help with the other ones.
Trying to do arc patch D9556
and arc diff prompts for 3 others...

From an Arcanist point of view, you did fine ;) However, the format of the author field in Git should normally be "Firstname Lastname <mail@domain>", please correct that.

Also, you could rebase your patch on latest master:

git checkout arcpatch-D9567
git fetch
git rebase origin/master
# Check if it still compiles
arc diff

For your other Diffs, I'll add instructions in a minute. After you followed them, Martin should be able to land all of those patches on your behalf.

In D9567#230718, @rkflx wrote:

For your other Diffs, I'll add instructions in a minute.

Done, and sorry for all the notifications. Those dependent revisions are sometimes tricky, but with Arcanist there should be no problem now anymore. If you are quick, this will hopefully be reviewed and landed shortly, to avoid having to rebase again…

Good luck ;)

Is there something I need to run before

git checkout arcpatch-D9567

arcpatch-D9567 is unknown to git

rkflx added a comment.Mar 27 2018, 7:54 AM

Is there something I need to run before

git checkout arcpatch-D9567

arcpatch-D9567 is unknown to git

Looking at Diff DetailBranch, your local checkout is apparently called arcpatch-D9567_1.

This revision was automatically updated to reflect the committed changes.