[3.0]: Config::getTempDir() is defaulting to sys_get_temp_dir about smf HOT 38 CLOSED

With all of that said. I would still expect it to use upload_tmp_dir before sys_get_tmp_dir();

I disagree. For a general purpose temp dir, it makes sense to me to use the... temp dir.

The activities aren't necessarily related to uploads. for the same reason, I wouldn't stuff it in the session temp dir.

If you want to override it, you can, that's your choice.

Note I did a couple searches on "vscode" and "git corruption" and of course there were multiple hits.... All eerily similar to this thread...

from smf.

FYI, The sys_get_temp_dir is what is recommended by PHP team. We pick the most preferred to the least preferred. Based on the system having the most optimal configuration for temp files up to the cache directory. Which may be configured using faster drives on high workload environments.

from smf.

I use linux. The logic makes sense to check until it finds a directory that exists and is writable. That then becomes the temp directory we work with. If the directory is showing as writable but isn't, I will argue that PHP or the OS itself is at fault. It is using the PHP standard calls, which I suspect are the standard C calls.

What is going on here is likely caused by Windows permissions, which are more complex than what is called by PHP. I would ensure the owner of the files has full permissions and that everyone has the permissions you expect. The file owner and "group" is determined by the process running PHP CGI and the primary group of that user (I believe).

The logic for finding the temp directory should live in Sapi, but due to constraints on the autoloader not being fully loaded when Config is started, it prevented it from working. Maybe someday we will get it over there.

from smf.

I think the problem is step no. 1 in steps to reproduce...

I use WAMP extensively & have never seen this. I literally have a dozen standing test SMF environments, and spawn new ones often for testing different bugs/configs/php vers/mysql vers/mysql engines.

I suspect reported issue is due to running a test environment directly on top of the live git repo - which IMO is an extremely questionable - and constraining - practice.

from smf.

Sidenote - never had a problem with this:

I'm just using my current WAMP defaults.

Sidenote sidenote - I changed WAMPs a while back, my prior one was falling into disrepair/lack of maintenance (which was sad, because I liked it a lot, very simple...). In its final days it forced me to setup an explicit temp dir on a drive - nothing else worked. One of the reasons I moved on was such random breakdowns. (Old was EasyPHP, new is wampserver...)

from smf.

Like @sbulen, I keep my repository and my live installation separate. It's much cleaner that way; no risk of getting files from the live installation into the repo.

Literally what .gitignore is for is it not?

Sure. Until you forget to add something to it. Keeping the repo separate from the live copy avoids human error. But whatever, you do you. 🙂

from smf.

Another question.

Shouldn't this be in Sapi instead of Config? There is a method for it in Sapi that just proxies to the Config method...

from smf.

It stops after one iteration if and only if the directory returned by sys_get_temp_dir() exists and is writable. Using switch vs. match doesn't make any difference here.

If using the system's temporary directory on Windows is creating issues because of how Windows handles file permissions, and if this is a common configuration on Windows, then we might want to change it at least for Windows. I must say it seems very stupid strange to me that permissions would be inherited from the directory in the way you describe, but maybe that's just how Windows does things.

At any rate, I think @live627 and/or @jdarwood007 use Windows, and @jdarwood007 is the one who originally wrote this code, so it is probably best discussed by you folks. There's very little I can do to help with Windows-specific issues.

from smf.

I think the problem is step no. 1 in steps to reproduce...

I use WAMP extensively & have never seen this. I literally have a dozen standing test SMF environments, and spawn new ones often for testing different bugs/configs/php vers/mysql vers/mysql engines.

Yea, me too. And?

I suspect reported issue is due to running a test environment directly on top of the live git repo - which IMO is an extremely questionable - and constraining - practice.

How so? It's literally a local environment. I would go so far as to say doing it any other way is a huge waste of time and very much constraining. Matter of fact. How else do you propose that I have git and xdebug both working from my ide and it break on a browser reload locally? It's literally designed to work that way. The entire reason for the vscode launch.json:
"pathMappings": { "C:/htdocs/event-prototype/public/index.php": "${workspaceFolder}/public/index.php" }

VScode also manages my local git repo. The only issue there would be the webserver reading and writing to the local git repo and since its literally an entire directory tree checked out (thats the whole point of git) there is no difference from apache/php reading/writing there vs my IDE. It literally should be transparent, which it is until something like this screws it up.

I mean I'm always up for learning something new and a better way so how do you do it? You have you repo in one directory and then do work, and then copy all of those files into a docroot?

I use linux. The logic makes sense to check until it finds a directory that exists and is writable. That then becomes the temp directory we work with. If the directory is showing as writable but isn't, I will argue that PHP or the OS itself is at fault. It is using the PHP standard calls, which I suspect are the standard C calls.

If it actually does the check correctly, and since I have other directories not only available but also writable then why does it not use one of those? Even in linux it should use any other writable directory before that one. I mean the comments in the code even allude to this.

// Fall back to sys_get_temp_dir even though it won't work, so we have something. if (empty(self::$temp_dir)) { self::$temp_dir = sys_get_temp_dir(); }

Its at the OS level. The temp directory and everything in it is created in a way that requires an Administrator account in windows to modify them or delete them. Due to the required permissions to write to that directory. In windows I would argue that php should not be able to write to that directory period since php should have its own tmp directory correctly configured. My system does, via wampserver but that directory is never used because the code falls through to, or overrides (cant recall, it should be in the original post) and it never attempts to use any other directory.

Even in linux ownership and permissions can be set as "sticky" so that all future directories/files inherit those same properties. If this is the case and the sys_get_temp_dir() is used in a virtual hosting environment, then once those files are copied over (if they can be copied over) the same situation could possibly exist could it not? We do not want files being copied over that can not be managed by the users account. I've not tested it in Nix due to not having the local resources to run docker or WSL2 well enough to be productive.

Run it through xdebug. On windows. It never considers another directory in windows other than sys_get_temp_dir(); Maybe I missed something, but I do not think so. If the code worked as expected, even in windows it should use upload_tmp_dir or session.save_path before using sys_get_temp_dir();

With the way this is indexed:
$temp_dir_options = [ 0 => 'sys_get_temp_dir', 1 => 'upload_tmp_dir', 2 => 'session.save_path', 3 => 'cachedir', ];

When the foreach is run which is it going to check first?

from smf.

I will also add. Since I never run wampserver as an administrator, for this very reason and since php is running with it, not sure how or why the server or php can even write to that directory in the first place. But it does. When they are copied back is the issue because they retain the ownership/permission from when they were written.

Git flaking out only highlights the issue. Well, I guess I should say that Git flaking out only highlights that sys_get_temp_dir() is checked first instead of last.

from smf.

Even in linux it should use any other writable directory before that one.

Just the opposite. That's exactly the directory that should be used for *nix systems. That's what it's for. The others are fallbacks in case the system's temporary directory isn't available for whatever reason.

But if it is possible to run into permissions issues on Windows for whatever reason, we'll need to address that.

from smf.

Even in linux it should use any other writable directory before that one.

Just the opposite. That's exactly the directory that should be used for *nix systems. That's what it's for. The others are fallbacks in case the system's temporary directory isn't available for whatever reason.

But if it is possible to run into permissions issues on Windows for whatever reason, we'll need to address that.

Ah, I suppose they would be the same in Nix.

I think the simplest solution would just be to reverse the search order in windows.

I will also note that this is only dev environment specific (so far) since the actual build continues to run fine. But, then again I have never ran it to a point where there is a follow up update to those same files, which might then be an issue.

from smf.

On those files copied over, do they retain the original C:\Windows\Temp permissions with inheritance removed?
Ensure that permission inheritance is configured properly.

You should see the same problem with attachments if permissions are not setup right since the files are uploaded to a temp directory and then moved to SMF. While not a git file, you should see permission issues.

from smf.

The configured upload_tmp_dir and the Windows TEMP directory are two different locations are they not? Yes, they very much are on my system:

The configured upload tmp:
c:/wamp64/tmp

One could argue since this directory is configured the aforementioned routine SHOULD absolutely use this path in a windows environment before using sys_get_temp_dir();

Since I routinely upload images and files during development, via the script and it causes zero issues even in GIT. It serves as confirmation (maybe, although its certainly supports it) that this routine should not use the sys_get_temp_dir() as its first choice in a windows environment.

from smf.

The configured upload_tmp_dir and the Windows TEMP directory are two different locations are they not?

That depends on your particular server's configuration. By default, PHP's upload_tmp_dir INI setting is null, and thus falls back to the system's temporary directory, which on Windows would indeed be C:\Windows\TEMP.

from smf.

The configured upload_tmp_dir and the Windows TEMP directory are two different locations are they not?

That depends on your particular server's configuration. By default, PHP's upload_tmp_dir INI setting is null, and thus falls back to the system's temporary directory, which on Windows would indeed be C:\Windows\TEMP.

I understand that. My point is that in a windows environment it will/should always be set to a directory, such as the one in my case that is configured for the webserver and php to write and read from cleanly. IE the webserver will own it. The webserver in windows will never own the system temp directory in windows. It will also always inherit those permissions when the files are created there.

If the system is configured that way then there is absolutely nothing we can do to fix it, other than creating a directory to write too. Which would be a better solution than the current one. If its created by the webserver, even then, in certain configurations it can cause issues in Nix if the webserver is incorrectly configured and does not run as the users account. However, there is only so much we can do to mitigate this in the real world. We do not control server config. But when its as simple as wrapping an array in a OS version check and reversing an array so that other paths are checked and used first. Well.....

from smf.

Going back to this question:

I mean I'm always up for learning something new and a better way so how do you do it? You have you repo in one directory and then do work, and then copy all of those files into a docroot?

Like @sbulen, I keep my repository and my live installation separate. It's much cleaner that way; no risk of getting files from the live installation into the repo.

I just use good old rsync to update my live installation whenever I am ready to test some set of changes. There's surely something similar to rsync available for Windows.

from smf.

Like @sbulen, I keep my repository and my live installation separate. It's much cleaner that way; no risk of getting files from the live installation into the repo.

Literally what .gitignore is for is it not?

from smf.

I just attempted to reproduce, using exactly the steps outlined above, and could not reproduce. The commit was generated just fine.

Win 10, wampserver, php 8.3.8, mysql 8.4. Using the latest 3.0 commit. Temp is the normal temp dir, as depicted in my screenshot above.

Note there isn't actually a description of the problem above, just "screws up my repo". More details of the actual issue would be needed - screenshots, error text, etc.

Because it all seems to work just fine.

I even acted like a madman and committed directly into the default branch. Somehow I survived.

from smf.

And I take it that one of the files was actually updated in Unicode? They do not update often and I imagine the most recent version has already been included in a commit. Just a hunch.

from smf.

This actually required some work, because the files were already current....

I first copied over some old unicode files from the 2.1.4 install, to ensure they were out of date. I ran the script, which ran to completion and updated the files.

But then the files matched current 3.0 repo.... To trigger a commit, I made some manual edits.

The commits were made.

So.... Can you provide more info on the issue you were seeing? It cannot be independently reproduced.

from smf.

I wonder if it could be due to me using vscode to make commits....

from smf.

Without an actual description of the problem, I can only wave my hands and raise my eyebrows.

from smf.

I will when I can. But it will require me to setup a clean build for this. Might take a few days. However, its literally exactly how I outlined in the OP.

Any time a Unicode file was updated Git would lose the right to commit (using vscode to manage the repo).

Since @sbulen and I run the same OS, and the same local server I would expect that our server environments are at the least very close. I have made zero customizations to wampserver so I know that is not an issue.

The next time I encounter it I will post the GIT log.

With all of that said. I would still expect it to use upload_tmp_dir before sys_get_tmp_dir();

from smf.

, I wouldn't stuff it in the session temp dir.

Agree. Not really sure why that was an option, but.....

My point is still valid. If upload_tmp_dir is valid and writable then its a better solution and should be used before sys_get_temp_dir() imho. Since this is where ALL uploads are saved anyway, which is the majority of writes by the webserver in general.

The activities aren't necessarily related to uploads. for the same reason, I wouldn't stuff it in the session temp dir.

True, but the search is just for a writable directory. If upload_tmp_dir is not set properly they are going to have bigger problems than just this issue.

from smf.

I would suggest closing this. There is no alignment on changing the temp dir, there were no issues encountered following the steps outlined above, and there are no details provided regarding what the actual errors were.

If an attempt is made to reproduce going forward, I suggest doing it manually outside of the vscode environment just to eliminate that as a factor. Just use plain SMF & git. And please provide details - screenshots, logs, lines of code, etc.

from smf.

Your suggestion of not considering vscode issues is a alienting factor to a large number of developers that utilize a similar workflow as I do.

I would say that number is pretty high as we expect a .gitkeep and .gitignore strategy that support our locals.

As long as I have been involved here do you really think I would be wasting your time for nothing?

Not a chance.

from smf.

The issue may not have anything to do with vscode. It may be any of a number of related issues, though - how it's configured, how it interacts with other tools installed, etc., etc.

And we would learn a lot if you repeated the process without vscode. If it works, that's a major clue. It's the most basic diagnostic approach to narrow down root cause, not a "don't use vscode" edict.

Also... I spent a few hours today trying to emulate your environment, despite expressing my own concerns above. Please think about that before saying you won't take further steps to attempt to isolate your problem, that at the moment, appears to be unique to your environment.

from smf.

I do not believe VSCode is the issue here as it should be using the underlying environment as the user to issue the git commands. If VSCode caused a problem, using git command line should also cause issues.

I believe this is entirely a configuration issue with Windows permissions. Its why nobody else has seen it either.

from smf.

I think it's fair to say that reproducibility is required before taking action, just like with any other issue. We need to know why this happens to @tyrsson but not @sbulen. Otherwise, we're shooting in the dark and any proposed changes are just avoiding and masking whatever the real problem is.

If the answer is simply that @tyrsson has some funky file permissions going on, then we need to know whether that's something that is likely to happen on a non-negligible number of production servers. Further, are there any tests we could perform to determine whether the necessary conditions apply? Until we have reproducibility, we don't know.

from smf.

Just as a FYI. I have changed zero things when it comes to file permissions. Which is why I do not understand this. At all.

No other application I work with has EVER caused an issue like this. That includes Wordpress. Not to mention no other file operation I have EVER performed has caused an issue like this. None. Zero.

But, there again, I never try to use the TEMP dir on windows. No matter what the server if I need a temp directory I create one. Then remove it on cleanup. Usually at ,/data/$dir

from smf.

I'm coming back to the fact that you are using your repository's working tree itself as your live install.

I assume that the web server is running under a different account than your own user account (which is to say, that your system isn't bats—t f—ing crazy). If so, then in all likelihood the repository files are owned by your user, whereas any files created by a PHP script executed by the web server would be owned by the web server user.

In that scenario, it would not be surprising at all if file permission problems arose when the generated files were moved into the working tree. Git pays attention to file permissions unless explicitly configured not to.

If this hypothesis is correct, it would explain why @sbulen does not encounter this issue: there are no file permission/ownership conflicts when files generated by the web server are never mixed with files in the working tree. It would also mean that there are two options available for you to resolve the issue in your local environment, @tyrsson:

1. Separate your repository's working tree from your live install as described above.

2. Adjust your Git configuration to ensure that file ownership and permissions are ignored. It's been a while since I last looked at that, but I think that you'll probably need to play around with a few different settings in order to get that working the way you want.

I recommend the first option. It's cleaner, simpler, and bulletproof.

from smf.

If this hypothesis is correct, it would explain why @sbulen does not encounter this issue:

He tried to replicate my exact setup and reported no issues. Let's not spend any more time on it for now. Next time it occurs I will collect all the information I can on it and post it up.

Git pays attention to file permissions unless explicitly configured not to

Then why do file uploads not cause any issues?

from smf.

Then why do file uploads not cause any issues?

The attachments directory is listed in .gitignore, so Git doesn't care about the ownership and permissions of any files in it.

from smf.

I mean in any other application I work in. I do not just work in SMF. Matter of fact that is the application I spend the least time working in these days. Well, in an actual SMF install that is.

No other application I work with causes these issues. Not the ones I build. Not the ones I work with.

WordPress,
XenForo,
Magento,

Matter of fact by your own logic the Unicode files should not matter either since they are on .gitignore as well. BUT THEY MATTER because of where they are created FIRST. Attachments are first created in the upload_tmp_dir.

from smf.

I wonder if @sbulen is running as an Administrator?

from smf.

WordPress also prefers to use sys_get_temp_dir() over upload_tmp_dir.

https://github.com/WordPress/WordPress/blob/df598e1d981f579cd0eebeec8eb5a343ef7fd03e/wp-includes/functions.php#L2202-L2244

from smf.

Let's not spend any more time on it for now. Next time it occurs I will collect all the information I can on it and post it up.

This seems like the best course for now. Feel free to reopen this issue once you have information to allow others to reproduce the issue.

from smf.