Hard Light Productions Forums

Modding, Mission Design, and Coding => FS2 Open Coding - The Source Code Project (SCP) => Topic started by: m!m on February 07, 2017, 10:42:09 am

Title: Possibly breaking filesystem change
Post by: m!m on February 07, 2017, 10:42:09 am
A recent change (https://github.com/scp-fs2open/fs2open.github.com/pull/1187) has introduced a new filesystem check that validates if the case of the name of the data directories is correct as defined by the engine. This does only affect development files and not VP files so the number of affected mods should be relatively small (I know that the development version of Diaspora has this issue).

The reason this check has been introduced is that while such a configuration will work on Windows, it will not work with Linux or Mac (or any OS which has a case sensitive file system). To fix it, you simply have to rename all the data directories to use the lower case version of the name (you might need to rename it twice since earlier version of Windows doesn't like renaming files where the old and new name only have case differences).
Title: Re: Possibly breaking filesystem change
Post by: Spoon on February 07, 2017, 12:55:39 pm
First off, thanks for making a thread. Not being informed about changes has been one of my biggest irks with the scp.

To clarify, so I got this straight. Because Unix is a OS for dirty peasants, the data folder can't start with a delicious capital D, but it has to be lower case? What about every other folder inside the data folder? All my folders start with caps, because my ocd demands it.

TBH I don't quite agree with the notion that breaking mods for this is 'justified'. I don't develop with unix, and if the game doesn't care either way when the files are in a VP, then why force this change upon us windows using devs? I don't really mean to make a big issue out of this or anything, I just find it kind of annoying.
Title: Re: Possibly breaking filesystem change
Post by: Phantom Hoover on February 07, 2017, 01:35:44 pm
Unix users still unpack VPs for development sometimes, and it's terrible user experience if that makes everything break.
Title: Re: Possibly breaking filesystem change
Post by: chief1983 on February 07, 2017, 01:49:50 pm
I understand it is a pain, but the pain on your side is rename some files, the pain on a case-sensitive filesystem user's end is not being able to run the content when not in a VP file.  Some mods release without VPs, etc.  Telling people that this shouldn't be done because some users will have breakage seems like the best solution to me, in this case.  From a developer standpoint, we have to try to avoid existing within our own bubble and realized this is a project used by users, some in a minority, of many different platforms.  Maintaining good compatibility with those platforms is often a big draw of talent to the development pool, as there are many Linux and Mac users out there that look for projects that cater to more than the PC Master Race :)
Title: Re: Possibly breaking filesystem change
Post by: Spoon on February 07, 2017, 02:02:15 pm
I understand it is a pain, but the pain on your side is rename some files, the pain on a case-sensitive filesystem user's end is not being able to run the content when not in a VP file.
Whats stopping the case-sensitive filesystem user from renaming the content when its not in a vp file?
Actually, renaming these thing is going to be a really annoying pain in the ass, because it breaks the svn folder structure. 
I really wish we could like, not do this. This has been a known issue for years now, why change this now all of the sudden? Or like, why not just have this dead stop error message on just linux and mac builds? I don't see why it would need to affect windows builds too.

Also, still waiting on that clarification if its just the data folder or every folder.
Title: Re: Possibly breaking filesystem change
Post by: The E on February 07, 2017, 02:08:02 pm
I think Spoon has a good point here: an Error might be overkill in this case. For the purposes of this, a one-time warning whenever a given directory is accessed (or even just a log print) should be sufficient.
Title: Re: Possibly breaking filesystem change
Post by: Phantom Hoover on February 07, 2017, 02:13:18 pm
It seems like the only reason for doing that would be so that users can indefinitely delay fixing their development files.

Spoon: why is renaming directories in SVN such a hassle?
Title: Re: Possibly breaking filesystem change
Post by: chief1983 on February 07, 2017, 02:17:25 pm
Well, not all mods have packaged their files, is one reason an error might be preferred to a warning.  If all mods ever were distributed in VP files and end users never had to mess with it, we could just log a warning.  But that's not the case.  Expecting users of a mod to go through the hassle of renaming files that they wanted to download and play with (every user that runs into the issue) vs the developer of a mod just doing a one time fix, seems like putting the burden of work in the wrong place doesn't it Spoon?  I don't understand the fix in question enough to know if it's all folders or just data, sorry.
Title: Re: Possibly breaking filesystem change
Post by: Spoon on February 07, 2017, 02:25:25 pm
I think Spoon has a good point here: an Error might be overkill in this case. For the purposes of this, a one-time warning whenever a given directory is accessed (or even just a log print) should be sufficient.
Yes, thank you. Just a log print should be enough.
Don't throw unnecessary warning messages or literally stop me from developing with an error message because I'm not developing in a unix compatitible way, which is literally irrelevant to me as the dev, and barely relevant to the end user because the end user is unaffected either way because of the VP's. And if they want to go ahead and unpack for whatever reason and do development things, then they can rename the folders themselves, surely?
This is really kind of a dumb thing to force this way, guys.

PH: Because the svn is case-sensitive, and since all my folders have caps, it means I have to delete all the old folders and files, then do a whole new checkout on every single file , and upload several gigs at the amazing speed of 80kb/s, because thats as fast as the svn allows me to upload, ever.
No thanks.

Well, not all mods have packaged their files, is one reason an error might be preferred to a warning.  If all mods ever were distributed in VP files and end users never had to mess with it, we could just log a warning.  But that's not the case.  Expecting users of a mod to go through the hassle of renaming files that they wanted to download and play with (every user that runs into the issue) vs the developer of a mod just doing a one time fix, seems like putting the burden of work in the wrong place doesn't it Spoon?  I don't understand the fix in question enough to know if it's all folders or just data, sorry.
Effectively all major mods ever have been distributed in VP files. The only one I can think of that has not done that, is Dimensional Eclipse, and DE shipped its files with lower case so its still not relevant. Making this into error (or a reoccuring warning) only serves to **** me over. Because please, feel free to point me to any recent relevant case in which an end user on a mac or linux system has ran into the problem of a mod they tried to run having a Data folder, and thus being unable to play it.
I bet you can't, because its not a common occurence at all. Because as I just said, effectively all mods, that are campaigns, get distributed in vp's.

This has effectively never been an issue of end users getting ****ed by a capitalized Data folder. This is just a thing that is randomly decided to be forced because it came up as an issue with the Diaspora dev team on github recently. And now Linux modders go "Yeah! We should totally force in the most heavy handed way possible because reasons!"
No, don't. This is dumb.
Title: Re: Possibly breaking filesystem change
Post by: m!m on February 07, 2017, 02:44:33 pm
What about every other folder inside the data folder? All my folders start with caps, because my ocd demands it.
Every folder has to use the case that the engine expects. Currently, the engine expects all directories to use the lower case names.

I think Spoon has a good point here: an Error might be overkill in this case. For the purposes of this, a one-time warning whenever a given directory is accessed (or even just a log print) should be sufficient.
Yes, thank you. Just a log print should be enough.
Don't throw unnecessary warning messages or literally stop me from developing with an error message because I'm not developing in a unix compatitible way, which is literally irrelevant to me as the dev, and barely relevant to the end user because the end user is unaffected either way because of the VP's. And if they want to go ahead and unpack for whatever reason and do development things, then they can rename the folders themselves, surely?
This is really kind of a dumb thing to force this way, guys.
The reason I chose to make this an Error is that on Unix a misnamed data directory breaks the game. FSO can't be run with those directory names on those platforms so we have to make it an Error there. Since we generally try to make all Warnings and Errors  consistent across all platforms I decided that Windows should also use an Error. If it's actually an issue to just rename the directories then we could make it a Warning or a log print on Windows.

However, you shouldn't ignore that Warning or Error since it would make it impossible for someone on Linux to run your development files. If, for example, you found an engine bug in your mod and wanted me to fix it using your development files, I wouldn't bother looking at the issue simply because I can't run your mod. It's in your own best interest to use the folder names that have the best compatibility across platforms but if it's a huge deal for mod developers we could lower the severity on Windows to a Warning. A log message would probably also work but most people just ignore that so it's less than ideal but considering that you would probably encounter ~20 Warnings if all your directories are misnamed it may be our only solution here.

This is just a thing that is randomly decided to be forced because it came up as an issue with the Diaspora dev team on github recently. And now Linux modders go "Yeah! We should totally force in the most heavy handed way possible because reasons!"
No, don't. This is dumb.
No one "forced" this change. I submitted this because I identified an issue and thought that it would not affect anyone. That's exactly why we have nightly builds so that we can quickly determine that a recent change broke the game for someone.
Title: Re: Possibly breaking filesystem change
Post by: AdmiralRalwood on February 07, 2017, 03:19:42 pm
I would personally prefer that FSO on case-sensitive filesystems just be willing to read from any capitalization of the folder structure. Don't we already do this for mod folders because of the MediaVPs?
Title: Re: Possibly breaking filesystem change
Post by: m!m on February 07, 2017, 03:25:40 pm
I already implemented (https://github.com/scp-fs2open/fs2open.github.com/pull/1184) that but I was told that we don't actually support upper case folder names so instead I built the current solution. I am not sure if the solution I presented in that PR is actually working since I needed to make some changes to the core CFile data structures.

Mods are different because they are handled by the command line system before CFile is initialized.
Title: Re: Possibly breaking filesystem change
Post by: Spoon on February 07, 2017, 03:39:18 pm
The reason I chose to make this an Error is that on Unix a misnamed data directory breaks the game. FSO can't be run with those directory names on those platforms so we have to make it an Error there. Since we generally try to make all Warnings and Errors  consistent across all platforms I decided that Windows should also use an Error. If it's actually an issue to just rename the directories then we could make it a Warning or a log print on Windows.
Yes, I am aware that it breaks the game for Unix. What I can't understand the logic of, is that, this has always been the case, so why suddenly break the game for windows? This literally accomplishes nothing. It just breaks the game for one additional platform now.

However, you shouldn't ignore that Warning or Error since it would make it impossible for someone on Linux to run your development files. If, for example, you found an engine bug in your mod and wanted me to fix it using your development files, I wouldn't bother looking at the issue simply because I can't run your mod. It's in your own best interest to use the folder names that have the best compatibility across platforms but if it's a huge deal for mod developers we could lower the severity on Windows to a Warning. A log message would probably also work but most people just ignore that so it's less than ideal but considering that you would probably encounter ~20 Warnings if all your directories are misnamed it may be our only solution here.
Yes, I'll keep this in mind for the first time it will happen that a Unix coders will go through the effort to try and fix a WoD bug. Usually case-sensitivity has not been the roadblock here.
Since in all my years developing WoD, the count of coders looking through WoD files has been pretty abysmally low. I usually just resign myself in having to create some test case in retail. But hey, I'll keep it in mind, who knows if this scenario will ever happen. Hell, I couldn't even get a mac build for WoD when I asked for it, repeatedly.
Either way, a log warning or just have fso not be picky about case-sensitivity would be very preferable over breaking the windows build.

No one "forced" this change. I submitted this because I identified an issue and thought that it would not affect anyone. That's exactly why we have nightly builds so that we can quickly determine that a recent change broke the game for someone.
I meant forced as in, this thing being forced upon me.
Title: Re: Possibly breaking filesystem change
Post by: AdmiralRalwood on February 07, 2017, 03:42:48 pm
I was told that we don't actually support upper case folder names
Except FSO handles it inside VP files just fine, so clearly we do support it; we could just extend that support outside of a VP archive.
Title: Re: Possibly breaking filesystem change
Post by: m!m on February 07, 2017, 03:49:58 pm
Except FSO handles it inside VP files just fine, so clearly we do support it; we could just extend that support outside of a VP archive.
I have written the code to support different cases on Linux so with a bit of testing we could use that if the consensus is that we want to support that case.
Title: Re: Possibly breaking filesystem change
Post by: Spoon on February 07, 2017, 04:36:23 pm
I just don't get why the first line of thought was: "We've got 2 platforms this is bugged on and 1 platform on which this is not. Let's make sure it's not going to work on all 3 platforms."
Rather than: "We've got 2 platforms this is bugged on and 1 platform on which this is not. Let's make sure it's going to work on all 3 platforms."  :confused:
Title: Re: Possibly breaking filesystem change
Post by: Phantom Hoover on February 07, 2017, 04:42:33 pm
Trying to overlay a case-insensitive filesystem over a case-sensitive one is a bit of a kludge and liable to lead to interesting edge cases, I guess. It sounds like we already have a pretty good implementation of it though?
Title: Re: Possibly breaking filesystem change
Post by: karajorma on February 07, 2017, 07:26:22 pm
I'm going to have to agree with Spoon on this one. Having a warning pop up on debug builds is one thing but forcing every single developer who uses SVN to rename their folders and then go through the massive hassle of checking in / out all the changes on SVN is probably the wrong way to go about this.
Title: Re: Possibly breaking filesystem change
Post by: chief1983 on February 07, 2017, 09:11:23 pm
Just out of curiosity, is the svn headache really all that bad?  As I understand it, a few 'svn mv' commands are all that would be required, issued on the directories themselves, not individual files.   According to http://stackoverflow.com/questions/3941291/a-sane-way-to-rename-a-directory-in-subversion-working-copy anyway, it makes it look pretty painless.
Title: Re: Possibly breaking filesystem change
Post by: mjn.mixael on February 07, 2017, 10:21:00 pm
Just out of curiosity, is the svn headache really all that bad?  As I understand it, a few 'svn mv' commands are all that would be required, issued on the directories themselves, not individual files.   According to http://stackoverflow.com/questions/3941291/a-sane-way-to-rename-a-directory-in-subversion-working-copy anyway, it makes it look pretty painless.

I tried that one time to hopefully mitigate a significant file restructure... it still re-uploaded all 2GB of data in the folder I was working on.
Title: Re: Possibly breaking filesystem change
Post by: chief1983 on February 07, 2017, 10:36:06 pm
Well, that sucks, unless subversion has become smarter about that I guess that would indeed be unavoidable.
Title: Re: Possibly breaking filesystem change
Post by: Goober5000 on February 07, 2017, 11:43:25 pm
Except FSO handles it inside VP files just fine, so clearly we do support it; we could just extend that support outside of a VP archive.

I have written the code to support different cases on Linux so with a bit of testing we could use that if the consensus is that we want to support that case.

This would be my preference.

I ran into this exact problem on the FSO Installer, which was first brought to my attention by (IIRC) niffiwan.  Some of the MediaVP zipfiles used the "MediaVPs" capitalization and some used "mediavps".  On Windows everything would be extracted into the same folder, but on Linux you'd end up with some files in one folder and some in another, leading to half-working MediaVPs when the game was run.  Rather than ask the MediaVP folks to re-package their files, I made the Installer do a case-insensitive search for existing folders with the same name.

It would be nice if everyone standardized on lowercase, but not even Volition was consistent in this (FS1 capitalized the first letter of the folder while FS2 used lowercase).  Using a case-insensitive search is the more robust solution.
Title: Re: Possibly breaking filesystem change
Post by: m!m on February 08, 2017, 05:43:16 am
The breaking change has been reverted and I submitted the case insensitive solution: https://github.com/scp-fs2open/fs2open.github.com/pull/1204
Title: Re: Possibly breaking filesystem change
Post by: m!m on February 09, 2017, 05:42:56 am
The case insensitive solution has been merged.
Title: Re: Possibly breaking filesystem change
Post by: Spoon on February 09, 2017, 08:02:56 am
The best outcome  :yes:
Title: Re: Possibly breaking filesystem change
Post by: LaineyBugsDaddy on February 09, 2017, 03:39:08 pm
I noticed that someone talked about case insensitive over case sensitive. My first thought is, why would that even be necessary? Isn't that why there are functions that convert strings to upper and lower case? Of course, a solution has already been merged, so my thought on the matter is irrelevant, of course.
Title: Re: Possibly breaking filesystem change
Post by: The E on February 09, 2017, 03:51:35 pm
I noticed that someone talked about case insensitive over case sensitive. My first thought is, why would that even be necessary? Isn't that why there are functions that convert strings to upper and lower case? Of course, a solution has already been merged, so my thought on the matter is irrelevant, of course.

The problem is that Unix filesystems are strictly case sensitive. "data", "Data" and "DATA" are all distinct directories, and since a lot of file paths in FSO are hardcoded (for example, tables are always assumed to be in "data/tables"), if someone ships a mod that has everything in a directory named "Data", FSO will not be able to open those files.
Title: Re: Possibly breaking filesystem change
Post by: LaineyBugsDaddy on February 09, 2017, 06:34:48 pm
Ouch. Right. Forgot about that.