Better heuristic for finding the .xcodeproj directory #471

0xced · 2016-05-31T21:44:38Z

The "Make the Mac Great Again" plugin would not install because Alcatraz assumes that the .xcodeproj has the exact same name as the package. This may not be true, for example "Make the Mac Great Again" xcodeproj file is "MakeTheMacGreatAgain.xcodeproj" (without the spaces).

This new heuristic searches for all .xcodeproj and works fine if only one exists in the cloned directory.

I also refactor error handling for better diagnostics when things go wrong.

0xced · 2016-05-31T22:04:58Z

Ha, I should have looked at the pull requests first, #461 implements pretty much the same heuristic! But this pull request adds better error messages in addition to this new heuristic.

kattrali · 2016-05-31T22:45:08Z

Alcatraz/Installers/ATZInstaller.h

+static NSString *const ATZInstallerErrorDomain = @"ATZInstallerErrorDomain";
+NS_ENUM(NSInteger)
+{
+    ATZInstallerXcodeProjectNotFoundError = 666,


🔥 🔥 🔥

@kattrali is that good or bad fire? 😬

@guillaume-algis @kattrali knows fires, she has the best fires!

guillaumealgis · 2016-06-01T10:05:55Z

Ha! It's really close to what I was doing with #461 indeed :)

#461 was stalled because I didn't take the time to test that all packages (color schemes and templates included) still installed fine after the change. Did you test your implementation on all packages?

In particular, I'm a bit worried that in your case installNameFromPbxproj: did not change that much (in contrast to efd7888#diff-b22b9cdae9c200d1c9d03235e2928944L148). But then it's not fresh in my mind, so maybe this works fine 👍.
My bad, did not see e8390db 🙂

Anyway, appart a few things (I'll comment inline), I'd be ok to merge your PR instead of #461 (if it's tested against all packages 😉).

guillaumealgis · 2016-06-01T10:12:24Z

Alcatraz/Installers/ATZPluginInstaller.m

+            [allXcodeProjFilenames addObject:directoryEntry];
+        }
+
+        if ([directoryEntry isEqualToString:@".git"]) {


I understand the performance gain from skipping .git, but isn't it premature optimization? Did you encounter a real-world case where scanning the .git folder was a problem?

You are right, I changed it in 609b8f4.

0xced · 2016-06-02T17:46:04Z

I'd be ok to merge your PR instead of #461 (if it's tested against all packages 😉).

I’m not sure if you are serious or not. I don’t think testing against all packages will bring value. This PR is about getting more packages to install. If some are still failing to install after this PR is merged, we should handle the issues on a per package basis IMHO.

guillaumealgis · 2016-06-02T19:24:21Z

@0xced So sorry if I wasn't clear. I am serious, but the goal is to ensure that we don't break the install process for the plugins which are already working today.

If some are still failing to install after this PR is merged, we should handle the issues on a per package basis IMHO.

Agreed!

0xced · 2016-06-02T21:12:35Z

the goal is to ensure that we don't break the install process for the plugins which are already working today

I took care of keeping the current behaviour of returning the .xcodeproj as soon as found if it has the same name as the package, so the risk of breaking existing packages is extremely low.

supermarin · 2016-06-02T21:22:50Z

Thanks for the contribution @0xced! 🍺

👍 when @guillaume-algis is ok to merge

guillaumealgis · 2016-06-02T23:31:43Z

I hacked together an ugly script to install all packages on my machine. Will report tomorrow on the findings. Thanks for your patience @0xced!

jurre · 2016-06-03T00:09:22Z

If anything, it would be nice to make sure that we install more packages correctly this way

guillaumealgis · 2016-06-03T08:12:20Z

After merging this PR, the following packages will install correctly instead of failing:

Build Me Up
Fastlane-Plugin
Make the Mac Great Again
Refactor Code
XYZHappyCoding
ZLCheckFile

This does not break any plugin which previously installed, as expected 🎉

Full report here: https://gist.github.com/guillaume-algis/1da584f0964e94803056782394137e6e

One final thing before merging: I think this deserves a minor version number bump, not just a patch (I consider this to be more than just a bugfix), wdyt? 1.2.0?

jurre · 2016-06-03T08:19:04Z

Great! Looks like a good improvement then. I agree on the version bump.

👍

guillaumealgis · 2016-06-07T09:02:45Z

Hi @0xced, would you mind amending 47c562b with 1.2.0 instead of 1.1.19 and rebasing your branch onto master so we can merge this? 🍻
Thanks!

0xced · 2016-06-07T09:49:23Z

Done.

guillaumealgis · 2016-06-07T09:59:46Z

👍

Perfect, thank you!

guillaumealgis · 2016-08-22T20:08:40Z

@jurre @kattrali @supermarin what do you think? LGTM?

supermarin · 2016-08-23T04:09:00Z

👍

guillaumealgis · 2016-08-23T12:36:16Z

Thank you @supermarin!

@0xced do you mind rebasing on top of master once more? Thanks!

The "Make the Mac Great Again" plugin would not install because Alcatraz assumes that the .xcodeproj has the exact same name as the package. This may not be true, for example "Make the Mac Great Again" xcodeproj file is "MakeTheMacGreatAgain.xcodeproj" (without the spaces). This new heuristic searches for all .xcodeproj and works fine if only one exists in the cloned directory.

* Define constants for error domains and error codes * Bubble up errors and present them to the user instead of just logging them

This is probably premature optimization, so better have smaller code.

0xced · 2016-08-23T21:37:29Z

Sure, I just rebased.

guillaumealgis · 2016-08-23T21:44:18Z

👍

Ugh. We need to re-approve. @supermarin ?

Edit: And the comment-parsing by pullapprove doesn't seems to work for some reason. I approved manually on pullapprove.com 🤔

jurre · 2016-08-23T22:07:42Z

👍

guillaumealgis · 2016-08-23T22:09:12Z

Thanks @jurre ! 🙂

jurre · 2016-08-23T22:20:02Z

Thanks @0xced and @guillaumealgis :)

kattrali reviewed May 31, 2016
View reviewed changes

guillaumealgis reviewed Jun 1, 2016
View reviewed changes

0xced force-pushed the find-xcodeproj branch from 47c562b to 0690da8 Compare June 7, 2016 09:31

guillaumealgis mentioned this pull request Jun 13, 2016

Update README.md alcatraz/alcatraz-packages#533

Closed

0xced added 8 commits August 23, 2016 23:30

Set indentation settings to four spaces at the project level

58820cf

Use NSError for expected error instead of catching exception

a153a25

Better error handling

e962956

* Define constants for error domains and error codes * Bubble up errors and present them to the user instead of just logging them

Do not assume the .xcodeproj file has the same name as the package

0b4194d

Do not skip descendants when searching for .xcodeproj

a2a152f

This is probably premature optimization, so better have smaller code.

Truncate error only when presenting it

5d6525c

Update CHANGELOG

c46d1fa

0xced force-pushed the find-xcodeproj branch from 0690da8 to c46d1fa Compare August 23, 2016 21:33

guillaumealgis merged commit 665299f into alcatraz:master Aug 23, 2016

0xced deleted the find-xcodeproj branch August 23, 2016 22:08

This was referenced Aug 24, 2016

Plugin is not building in Alcatraz MakeZL/ZLCheckFilePlugin-Xcode#1

Closed

ZLCheckFile is not building alcatraz/alcatraz-packages#342

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better heuristic for finding the .xcodeproj directory #471

Better heuristic for finding the .xcodeproj directory #471

0xced commented May 31, 2016

0xced commented May 31, 2016

kattrali May 31, 2016

guillaumealgis Jun 1, 2016

supermarin Jun 1, 2016 •

edited

Loading

guillaumealgis commented Jun 1, 2016 •

edited

Loading

guillaumealgis Jun 1, 2016

0xced Jun 2, 2016

0xced commented Jun 2, 2016

guillaumealgis commented Jun 2, 2016

0xced commented Jun 2, 2016

supermarin commented Jun 2, 2016

guillaumealgis commented Jun 2, 2016

jurre commented Jun 3, 2016

guillaumealgis commented Jun 3, 2016

jurre commented Jun 3, 2016

guillaumealgis commented Jun 7, 2016

0xced commented Jun 7, 2016

guillaumealgis commented Jun 7, 2016 •

edited

Loading

guillaumealgis commented Aug 22, 2016

supermarin commented Aug 23, 2016 •

edited

Loading

guillaumealgis commented Aug 23, 2016

0xced commented Aug 23, 2016

guillaumealgis commented Aug 23, 2016 •

edited

Loading

jurre commented Aug 23, 2016 •

edited by supermarin

Loading

guillaumealgis commented Aug 23, 2016

jurre commented Aug 23, 2016

Better heuristic for finding the .xcodeproj directory #471

Better heuristic for finding the .xcodeproj directory #471

Conversation

0xced commented May 31, 2016

0xced commented May 31, 2016

kattrali May 31, 2016

Choose a reason for hiding this comment

guillaumealgis Jun 1, 2016

Choose a reason for hiding this comment

supermarin Jun 1, 2016 • edited Loading

Choose a reason for hiding this comment

guillaumealgis commented Jun 1, 2016 • edited Loading

guillaumealgis Jun 1, 2016

Choose a reason for hiding this comment

0xced Jun 2, 2016

Choose a reason for hiding this comment

0xced commented Jun 2, 2016

guillaumealgis commented Jun 2, 2016

0xced commented Jun 2, 2016

supermarin commented Jun 2, 2016

guillaumealgis commented Jun 2, 2016

jurre commented Jun 3, 2016

guillaumealgis commented Jun 3, 2016

jurre commented Jun 3, 2016

guillaumealgis commented Jun 7, 2016

0xced commented Jun 7, 2016

guillaumealgis commented Jun 7, 2016 • edited Loading

guillaumealgis commented Aug 22, 2016

supermarin commented Aug 23, 2016 • edited Loading

guillaumealgis commented Aug 23, 2016

0xced commented Aug 23, 2016

guillaumealgis commented Aug 23, 2016 • edited Loading

jurre commented Aug 23, 2016 • edited by supermarin Loading

guillaumealgis commented Aug 23, 2016

jurre commented Aug 23, 2016

supermarin Jun 1, 2016 •

edited

Loading

guillaumealgis commented Jun 1, 2016 •

edited

Loading

guillaumealgis commented Jun 7, 2016 •

edited

Loading

supermarin commented Aug 23, 2016 •

edited

Loading

guillaumealgis commented Aug 23, 2016 •

edited

Loading

jurre commented Aug 23, 2016 •

edited by supermarin

Loading