Opened 4 years ago

Closed 17 months ago

#12951 closed enhancement (fixed)

Allow modification of file permissions in wxFileName

Reported by: cacb Owned by:
Priority: normal Milestone:
Component: base Version: stable-latest
Keywords: wxFileName simple Cc:
Blocked By: Blocking:
Patch: yes

Description

There is currently (wx 2.8.11) no way to modify file attributes\permissions in a portable manner using wxWidgets.

It is suggested to add functions wxFileName::MakeReadable(bool) and wxFileName::MakeWritable(bool) to support adding and removing attributes in the style of chmod + and -.

Attachments (2)

12951_attempt1.patch download (10.8 KB) - added by catalin 17 months ago.
12951_attempt2.patch download (7.7 KB) - added by catalin 17 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 4 years ago by vadz

  • Component changed from GUI-all to base
  • Keywords wxFileName simple added
  • Status changed from new to confirmed
  • Summary changed from Allow modification of file attributes in wxFileName to Allow modification of file permissions in wxFileName
  • Version changed from 2.8.x to 2.9-svn

Changed 17 months ago by catalin

comment:2 Changed 17 months ago by catalin

  • Patch set

The attached patch is an attempt to implement this.
The new wxChmod()/wxCRT_Chmod() & co are similar to wxAccess()/wxCRT_Access().
Testing was done only under MSW. Compiled with VC9 and gcc4.7.1.

comment:3 Changed 17 months ago by vadz

Thanks for the patch but I'd really prefer to do it at wxFileName level. For me the wx-specific functions in wx/filefn.h should really be considered as morally deprecated even if they are not (yet) deprecated at the code level (wxCRT_Chmod() is just fine though). Could you please make the new functions wxFileName methods instead? TIA!

comment:4 Changed 17 months ago by catalin

It should be no problem, but it the approach ok? I used mode flags instead of the initially proposed bool.

comment:5 follow-up: Changed 17 months ago by vadz

wxSetFilePermissions() itself (which would presumably become just SetPermissions() once it's a member of wxFileName) is definitely fine. I'm not so sure about the two other functions, it's not really clear what do they do. I wouldn't be too upset to not have them at all to be honest, they create too many questions (why no "SetGroupWritable()"? what about "SetExecutable()"?) for the limited usefulness they provide.

One other thing: once the function(s) are in wxFileName, they could also handle symlinks correctly thanks to support for them in this class. I'm not sure if it's really worth it as symlinks permissions are mostly (always?) ignored anyhow but it could be done.

And, last and really least, there seems to be something other than plain single quote used in "doesn't" string in the documentation, could you please check it?

Thanks again!

comment:6 in reply to: ↑ 5 Changed 17 months ago by catalin

The two more specialized functions could be removed, though they are closer to the initial ticket description.

wxMakeFileReadable() would only [un]set read permissions for its target. The benefit would be that calling it will ignore write and execute permissions. It already supports setting read permissions separately for user/group/others. Also calling wxSetFilePermissions() with only read permissions for a writable file will make it read-only, so the user should know in advance if other permissions should be set or not.
Similar for wxMakeFileWritable().

Replying to vadz:

why no "SetGroupWritable()"? what about "SetExecutable()"?

The former is already supported; the latter may be added if requested?
Anyway, for now I'll create wxFileName::SetPermissions().

One other thing: once the function(s) are in wxFileName, they could also handle symlinks correctly thanks to support for them in this class. I'm not sure if it's really worth it as symlinks permissions are mostly (always?) ignored anyhow but it could be done.

I'll look into that.

And, last and really least, there seems to be something other than plain single quote used in "doesn't" string in the documentation, could you please check it?

Yes, odd, I'll replace them.

Changed 17 months ago by catalin

comment:7 Changed 17 months ago by catalin

New patch attached.

comment:8 Changed 17 months ago by vadz

This is definitely a good start, committing soon, thanks!

comment:9 Changed 17 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [74639]) Add wxFileName::SetPermissions().

This is a simple wrapper for the POSIX chmod().

Closes #12951.

Note: See TracTickets for help on using tickets.