Adding Gershwin filesystem layout and vars#62
Conversation
rfm
left a comment
There was a problem hiding this comment.
I have no problem with the basic filesystem layout stuff; seems fine to me.
Add review comment
Markdown input: edit mode selected.
Write
Preview
I'm not completely sure about the additional variables though (I think we may already have more than is ideal). It's not clear what the new variables are intended to be used for, so at the very least I think it would be good to add that documentation (it would be good to document existing ones too).
Have you considered that, when you add extra variables for installation locations in gnustep-make, you also need to add corresponding constants to NSPathUtilities in gnustep-base as well (one reason to be cautious about adding them)? The only reason I'm aware of for having extra variables is if you want/need things using them to be installed in some completely separate location rather than in a subdirectory. Otherwise it may be easier (and perhaps more managable) to simply defined/document a convention that utilities are in the 'Utilities' subdirectory of NSApplicationDirectory and that core services applications are in the 'Applications' subdirectory of NSCoreServicesDirectory, rather than adding GSUtilitiesDirectory and GSCoreServicesApplicationsDirectory to base.
That being said, I don't see any real problem, and indeed I think Apple added NSCoreServicesDirectory and we never fully implemented it, so adding that one at least seems like a good thing.
a7f49a5 to
dcdc3ec
Compare
dcdc3ec to
0f4dca3
Compare
|
Thank you @rfm I have pushed changes, hopefully this looks a lot better. Yes the purpose was to move certain things like WindowManager.app, Menu.app, LoginWindow.app out of /System/Applications but still have them work with openapp for convenience without a full path, to make nice GNUmakefiles, and to have these components install somewhere that feels familiar. Gershwin has a menu now which lists apps in all domains but we do not want WindowManager showing up there and so this also fixes that. I can confirm installing those things in CoreServices nicely via GNUmakefiles, running without fall paths works fine with my latest changes. I was not aware additional changes were required to libs-base but I now do see that CoreServices is already here in libs-base where you mentioned? https://github.com/gnustep/libs-base/blob/master/Source/NSPathUtilities.m#L2304 So with all of this I believe NSCoreServicesDirectory is complete now if I understand right? |
|
Actually @rfm after looking over libs-base again I see it only includes |
|
Please do add the missing domains. I would do it myself but am away from my computer this morning. |
Done, PR created gnustep/libs-base#576 |
|
Sorry, after I merged this pull I realised I'd made a mistake and had to revert it. The problem was the ...CORE_SERVICES stuff. I had failed to notice that we already have variables for this using the _SERVICES suffix rather than a _CORE_SERVICES suffix. Please could you regenerate the pull request using the existing variables. Sorry for the extra work. |
|
Thanks @rfm no problem I will take a look, and make a new PR. |
|
Hi @rfm it seems these existing vars will install to SERVICES not CORESERVICES. I think that would be fine. Interesting enough it doesn't seem the path's work like CoreServices did. When things were installed in CoreServices I could still openapp Menu and it would work. Not sure why it doesn't work the same with Services? Perhaps missing things in NSPathUtilities?
|
|
I've been talking it through with @probonopd and I think we are liking the idea to use Services vs CoreServices anyway. I'll get the PR ready. 👍 |

Hi @rfm a while back we discussed with @gcasa about contributing a Gershwin filesystem layout in one of the GNUstep meetings, and it seemed acceptable. I had a PR open previously but realized it was not going to work for additional variables we wanted to add.
This PR adds:
Would something like this be acceptable, or could we work out an easier way to introduce new variables via filesystemlayouts alone? Is this going to be too much? I couldn't find a way to do this without so far adding to everything shown in the PR, but please do let me know if there is a way to make this work touching less.