Fix client pid check #786
Fix client pid check #786
Conversation
X's Success return value is 0, not a boolean, so the check was effectively inverted.
Based off Metacity 's commit 7c1cc3ca1d8131499b9cf2ef50b295602ffd6112 [1]. [1] https://gitlab.gnome.org/GNOME/metacity/-/commit/7c1cc3ca1d8131499b9cf2ef50b295602ffd6112
|
Hello @cwendling, how are you doing? Do you know where I could find build instructions for it? I've never done it. Tks 👍 |
Try if you can reproduce with |
OK thanks, using this it fixes the issue: without these patches it shows "as superuser", with it it shows like a normal window. So that's a first win :)
It depends a bit on your distribution and how comfortable your are building things, but on Debian-based distros, you can do something like: ~$ sudo apt-get build-dep marco
~$ git clone https://github.com/cwendling/marco.git
~$ cd marco
~/marco$ git switch client-pid
~/marco$ ./autogen.sh && ./configure && make -j4 && sudo make installThis will install the patched To uninstall the local build, run It's pretty standard, so you can also find additional info a bit everywhere :) |
|
@cwendling Nice. Will try it 👍 |
|
@raveit65 I guess the issue you mentioned under X2Go was due to the lack of verification both that XRES was available, and that the XRES call was successful or not. Unfortunately although the XRES test implemented in #747 was good, the added check for the whether the XRES call was successful was done the wrong way around. Either way, this shouldn't bring back the X2Go issue as both checks are kept, only the latter is fixed so things actually work when XRES is present. And thanks for testing :) |
|
I cannot test this for function (no flatpack) but can check the build easily enough
|
lukefromdc
left a comment
lukefromdc
left a comment
There was a problem hiding this comment.
While I could not test with flatpak (not installed here), I didn't get any issues with the build, nor a problem with not showing caja under sudo as "ROOT" or applying that to caja running as a normal user
|
@caiodev have you had a chance to test this yet? |
|
Is this ready to go or are we still waiting on another review? |
|
I think this can get in, although I would have liked people that cared most in the original issue to test… but well. |
|
So sorry for my delay, guys. I tried to build it using @cwendling instructions but that's the output I got:
Processing ./configure.ac Before this attempt, I had to install mate-common and yelp-tools from official servers (Should they be from another source?) too but I couldn't build it to a working point Do I have to give those files execution permission? Btw, I went to clone your branch @cwendling but couldn't find one branch named client-pid so I cloned this repo and there it was. Would you guys have any idea what could be wrong on my end? Also, this command (sudo apt-get build-dep marco) gave me this output: Reading package lists... Done Much appreciated 👍 |
|
Just got a little bit further I think I installed autopoint but now it complains about glib-2.0:
Since the full log is lengthy, I put it into a text file I'll be attaching to this message 👍 Update: The last dependency missing for me is this: mate-desktop-2.0 but I couldn't find a way to satisfy it yet |
|
@caiodev Have you run Anyhow, for |
|
Commit is already in fedora f40/41/rawhide in a few days :-) |
Oh I see, @cwendling. I'll try that as soon as I can. I haven't had much time to test it, but I think I'll be able to do it till the end of the week max. Will try to do my best. Thanks for your guys' patience 👍 |
|
Hello guys. Just to update you on my attempts to test the solution branch, I really could not build it because the version of MATE I'm using is not compatible with the required one and I got this error:
I downloaded the latest version of MATE from their Github page as @cwendling suggested but since I own 2 PCs running Ubuntu MATE 24.04.1 and I always try to test anything MATE related (New releases etc) on baremetal because I think it is a more accurate environment, when I was about to start building it, I saw reports of some inconsistencies on the latest MATE vesion and since these PCs are shared by the whole family, I decided I couldn't go any further because of that and because I might mess with something that will take me some time to fix making at least one of those PCs unavailable for use for some time ): Since this PR has been open for 1 month now and @raveit65 has already tested it, imho we could go with his report on it but that's you guys' call. Finally, I'd like to thank you guys once again for your patience, for submitting/testing this fix and for all you guys do here for the community 🤝️ |
|
@caiodev if you're not afraid of testing binaries from the web, you're in luck today: I just built a patched Ubuntu 24.04 package with the changes form this PR applied: marco_1.26.2-4.1_amd64.zip |
Fixes #301.
Disclaimer: I didn't test this, I don't actually use flatpacks or such. However, given a check was clearly the wrong way around, and I imported another fix from Metacity, it might just work.
If this doesn't do it, possibly importing Metacity commits dade470e13292c58822b0c180019caf0ea3862f3 c6584a38a234726c1128d044fd7cc5a0f8379c69 c5136ec907fab167ffddfe4d10c3902c6c5d8852 8430e8436cb6bac732c00de5b5a3c3d73be62871 a7d6f11b3e712751b010a8f7622b148c93765f70 7c1cc3ca1d8131499b9cf2ef50b295602ffd6112 3e6358113acf61e0c68419bec6cc68a29603a2ec might help (it's mostly refactoring in the end tho, so I'm hopeful the simpler patch here would work).
CC @lukefromdc @lambdanil @tidux @rusty-snake @caiodev (and everybody else from #301), please test if you can :)