Skip to content

Fix fallback Gitify-Cache-Folder#176

Merged
alroniks merged 7 commits intomodmore:masterfrom
sebastian-marinescu:patch-1
Dec 9, 2016
Merged

Fix fallback Gitify-Cache-Folder#176
alroniks merged 7 commits intomodmore:masterfrom
sebastian-marinescu:patch-1

Conversation

@sebastian-marinescu
Copy link
Contributor

On my server I hit the last if condition. So Gitify tries to set the Gitify-Cache-Folder, named ".gitify", in the Gitify-Working-Dir, which also includes ".gitify" (the file). This failed, because the Gitify-Cache-Folder is the Gitify-Config-File.

This changes the Gitify-Cache-Folder to ".gitify-cache".

On my server I hit the last if condition. So Gitify tries to set the Gitify-Cache-Folder, named ".gitify", in the Gitify-Working-Dir, which also includes ".gitify" (the file). This failed, because the Gitify-Cache-Folder is the Gitify-Config-File.

This changes the Gitify-Cache-Folder to ".gitify-cache".
@alroniks
Copy link
Collaborator

Hi @sebastian-marinescu
Thank you for the pull request, but an issue in the other place. Goal was to make the same mechanism as in composer - put MODX archives to user home folder in special .gitify folder. So code in application work absolutely correct and this folder can be placed in gitify project root also.

But when trait DownloadModx try download modx it checks folder here https://github.com/modmore/Gitify/blob/master/src/Mixins/DownloadModx.php#L51 and the funtion file_exists returns false if file exists and destination folder for package will not created. So you see error.
A fast fix - add additional check to this rule:

if (!file_exists(GITIFY_CACHE_DIR) && !is_folder(GITIFY_CACHE_DIR)) {
    mkdir(GITIFY_CACHE_DIR);
}

@sebastian-marinescu
Copy link
Contributor Author

Hi @alroniks
thanks for your input and the explanation.
Added the fast fix and also checked if is_folder in the ClearCacheCommand.
What do you think? It's okay like that?

@sebastian-marinescu sebastian-marinescu changed the title Fix fallback Gitify-Cache-Folder Check Gitify-Cache-Folder Feb 20, 2016
@alroniks
Copy link
Collaborator

Hi @sebastian-marinescu
Sorry for a delay and my wrong advice. I double checked your PR and all behavior and found a solution. I can fix it, but I thankful to you for help and I not want to lose your contribution. Your first thoughts were correct.
Please, remove is_folder from ClearCacheCommand.php and DownloadModx.php. Really in PHP exists only is_dir, but now it doesn't matter.
And add to 'application.php' code from 48 line:

$cacheDir = '.gitify';
if (is_file(implode(DIRECTORY_SEPARATOR, array($home, $cacheDir)))) {
    $cacheDir .= '-cache'; // need in case when home directory is the working dir of gitify also
}

define('GITIFY_CACHE_DIR', implode(DIRECTORY_SEPARATOR, array($home, $cacheDir, '')));

After this fix, gitify will check .gitify file in root and if it exists, then it will try to create .gitify-cache folder instead .gitify. It's strange, but the truth: in many systems folder also a file, but with some differences. So we cannot keep folder and file with the same name in one folder.

@sebastian-marinescu sebastian-marinescu changed the title Check Gitify-Cache-Folder Fix fallback Gitify-Cache-Folder Feb 23, 2016
@sebastian-marinescu
Copy link
Contributor Author

Hi @alroniks and thanks for your reply!
I am very sorry for and embarrassed by is_folder - but you know....it was late 😜
I'll change the PR in the next couple of days and commit the fix 👍

@sebastian-marinescu
Copy link
Contributor Author

Okay, I think this should be okay now, but I haven't tested it on the aforesaid server.
Either you review it again, or I'll report back when I've tried it out 👍

@sebastian-marinescu
Copy link
Contributor Author

Hi @alroniks I tested the fix on two projects and it works as expected.
As I must use this fix on the servers I'm working I would love it, if we could merge this PR soon :shipit:

@sebastian-marinescu
Copy link
Contributor Author

Thank you for merging! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants