[Bf-committers] Accessing Files (new config locations, fopen wrapper, etc)

[Andrea Weikert]

Hi,

agree on these points, some comments inline.

Brecht Van Lommel schrieb:
> Also, resolving a file path with // in it depends on which .blend file
> the datablock is linked from, I don't think the proposed API takes
> this into account.
>   
yes, that needs to be kept working.
>> The functions getSystemDir and getUserDir are added to GHOST, which adds
>> a pretty nasty dependency to BLI_blenlib - have you any idea on how to
>> solve this?
>>     
>
> I also don't really see the reason to do this, there is already
> platform abstraction code in blenlib. If mac needs an objective-c .m
> file in blenlib I don't see why that wouldn't work. I'm not against
> GHOST becoming a platform abstraction library but this seems like a
> strange exception.
>
>   
+10 :)
>> There are quite a few functions that need consulidation: BLI_where_am_i,
>> BLI_where_is_temp, get_install_dir,  BLI_gethome_folder,
>> BLI_gethome_system, BLI_gethome_user and for Windows there's also
>> BLI_getInstallationDir...
>> In these functions you can also find the code that needs to go into the
>> windows version of getSystemDir and getUSerDir.
>> I think that a bit of planning needs to be done how to consolidate this
>> with your proposal, otherwise we'll just add more to the mess that is
>> alsready there.
>>     
>
> Agreed, probably BLI_bfile_fopen makes most of the calls to these
> obsolete, but e.g. scanning a directory for python scripts should use
> consistent functions.
>
>   
+1
apart from the cases where you'd have to access the paths unrelated to 
opening them, internally the BLI_bfile_fopen
will still need to call these functions (or similar ones).
My point was that if we do this, we should have one consistent api to 
access these files and not adding more functions to get
basically the same directories.

> I don't think setting environment variables like that is permanent?
>
>   
ok, my bad, just tested and yes, even Windows has these only while the 
process lives, so
that should be ok.

> Some more comments:
>
> * I don't really see the point of the "environment" file. In order to
> install blender in a custom location you would already need to set
> BLENDER_SHARE as an environment variable, why not simple set all of
> them then? In other cases it could just use defaults hardcoded in the
> executable. Another thing is that this file is platform dependent, so
> having the defaults hardcoded means we don't have to set up all the
> build system to copy the right files. It also means we can run the
> blender executable by itself, which is a feature I would like to keep.
> Basically I think this gives you two ways to do the same thing, and it
> may be more confusing than helpful, actual environment variables work
> well enough, we don't need to add a configuration file?
>
>   
+1 I think too this should be doable with a hardcoded default
> * It is not clear to me what the point is of having BLENDER_VERSION as
> an environment variable (or Blender setting any environment variable
> when it starts up). It would be set only while Blender is running, and
> for e.g. python scripts it's accessible in other ways already. Just
> store it as a variable somewhere in the code or use the internal
> #define?
>
>   
I think the point behind this is to be able to pass the blender folders 
to other applications for
pipeline integration.
> * I can see wrapping file open/close is useful but I don't see the
> purpose of wrapping read/write if you provide access to the FILE
> pointer anyway. Otherwise you end up with inconsistent code or
> wrapping all the functions, like fseek, fprintf, ferror, feof, fputs,
> .., better to not try this in my opinion.
>
>   
+1
I think wrapping file open/close should be enough to enable writing to 
temp first and only after
successful writing rename the file (and possibly remove old file, thus 
effectively overwriting the
old one). We do this for .blend files already, but I can see where it 
might be useful for other files
too.
> * Do we have a good reason to support both BLI_bfile_fopen and
> BLI_bfile_open syntax? Might just be historical reasons and we can use
> fopen syntax everywhere?
>
>   
+1
It's at least worth to take a look if we can get by with using fopen only.