[Pharo-dev] Working Directory Changes

Guillermo Polito guillermopolito at gmail.com
Thu Oct 12 05:53:15 EDT 2017


Hi all,

Now I had some minutes to take a further look at this.

** Mariano, yes, I remember that conversation. What we propose is aligned
to that:

FileSystem workingDirectory shouldBe == getcwd

And **almost always**

FileSystem workingDirectory shouldBe ==  $PWD

This is because the real working directory is the one held by the process.
$PWD is managed by bash and it can get out of sync with getcwd. For more
info take a look at rajula's blogpost in the first email.


** About security, I don't really buy the argument.
 - There are many aspects of security: internal in the image, external. FFI
is not the only thing to *care about*
 - right now we can do stuff like
    - FileSystem root ensureDeleteAll (and given pharo started with sudo...)
    - nil become: true
    - OpalCompiler compile: 'someMethodThatOverriddenBreaksEverything ...'
 - FFI plugin is shipped with the VM
    - so removing FFI support is as easy as removing the plugin
    - but this means that even if that FFI support is not in the image, any
piece of code can do:
         - Myclass compile: 'test Metacello install: ''FFI''; please'; test.
         - or create a method calling the FFI primitives

 This does not mean I don't think about security, but I believe it should
be managed in some other way. A security solution for real needs to manage
reflection as well as external resources, and control how dependencies and
packages are loaded. Because, moreover, I believe that while you may want
to forbid FFI calls in some part of your app, you may want that some other
parts of the system can use FFI.

** About modularity. But I'm sure I'm one of the guys that cares the most
about modularity and cries aloud every time we introduce bad dependencies.
Now, I'm proposing the introduction of FFI because I think its manageable
and it's being controlled. It does not affect the bootstrap, and I don't
think it will forbid us to get even smaller kernels.

** The most important argument, that at least I did not see anybody
complain about, is backwards compatibility. This implementation may break
backwards compatibility, because this does not hold anymore:

FileSystem imageDirectory == FileSystem workingDirectory

Well, yes, it holds if you start your vm/image from the same directory
where the image is located. Which is what most users do.

** I also talked with Esteban about his reasons. His concern was about
consistency. If you take a look at how getEnv is resolved:

getEnv: varName
"This method calls the Standard C Library getenv() function."
"OSEnvironment current getEnv: 'HOME' "
<primitive: 'primitiveGetenv' module: '' error: ec>
ec ifNil: [ ^self getEnvViaFFI: varName ].
ec == #'bad argument' ifTrue: [
varName isString
ifFalse: [ ^self getEnv: varName asString ] ].
self primitiveFail

If first tries to call a primitive and then, as fallback, it calls FFI by
default.

I changed the pull request to do the same thing about the working
directory. That way, as soon as we have implemented the primitive, that
will be in usage instead of the FFI call.

Guille

On Wed, Oct 11, 2017 at 3:07 PM, Denis Kudriashov <dionisiydk at gmail.com>
wrote:

>
>
> 2017-10-11 14:14 GMT+02:00 Alistair Grant <akgrant0710 at gmail.com>:
>
>> On Wed, Oct 11, 2017 at 01:32:52PM +0200, Denis Kudriashov wrote:
>> > 2017-10-11 10:54 GMT+02:00 Esteban Lorenzano <estebanlm at gmail.com>:
>> >
>> >     in general, this core-core functions is better to implement them as
>> >     primitives.
>> >     and you put a fallback using UFFI (in case the primitive is not
>> there).
>> >     of course, your solution works? but it adds dependency with
>> FFI-Kernel,
>> >     which is also not good to have it in the bootstrap IMO.
>> >
>> >     take a look at OSEnvironment>>getEnv implementation.
>> >
>> >     we can go that direction and I think it will be better.
>> >
>> >
>> > +1 for new primitive. Because instead any new platform will require
>> implemented
>> > FFI which is not easy.
>>
>>
>> Would it make sense to add this to FilePlugin?  The plugin has to be
>> working for general file system access anyway.
>>
>
> I think it is good idea
>
>
>>
>>
>> Cheers,
>> Alistair
>>
>>
>> >     Esteban
>> >
>> >
>> >         On 11 Oct 2017, at 10:27, Guillermo Polito <
>> guillermopolito at gmail.com>
>> >         wrote:
>> >
>> >         Hi all,
>> >
>> >         I'd like to push a really core change in file management: the
>> working
>> >         directory. This is really needed for command line apps, when
>> you have
>> >         your app deployed in some directory and you're launching it from
>> >         another one. The current implementation, where workingDirectory
>> =
>> >         imageDirectory, forces to have absolute paths or extra handling
>> all
>> >         over the place to manage this complexity.
>> >
>> >         Rajula proposed a couple of months ago a solution for this
>> based on the
>> >         getcwd functions. You can read in his blogpost why using getcwd
>> is
>> >         better than $PWD in general here:
>> >
>> >         https://vineetreddy.wordpress.com/2017/05/17/pwd-vs-getcwd/
>> >
>> >         Now, since accessing the working directory is a core part of
>> Pharo but
>> >         based on UFFI, his implementation was breaking the build
>> process. We
>> >         cannot and we will not integrate UFFI in the bootstrap because
>> it
>> >         depends mainly on the compiler which is a big beast. Instead, I
>> propose
>> >         that only for this core-core-core feature, we use directly FFI.
>> >
>> >         In other words, the bootstrap will include just a couple of
>> classes to
>> >         manage the basics of FFI. And the working directory will be
>> fetched by
>> >         using this low level API. Such a call looks like this:
>> >
>> >         (ExternalLibraryFunction
>> >             name: 'getcwd'
>> >             module: 'libc.dylib'
>> >             callType: 1
>> >             returnType: ExternalType char asPointerType
>> >             argumentTypes: {
>> >                 ExternalType char asPointerType.
>> >                 ExternalType long })
>> >                     invokeWith: buffer with: bufferSize.
>> >
>> >         We reviewed it with Pablo two days ago. The build process works
>> with
>> >         this implementation and tests are still running. The pull
>> request is in
>> >         here:
>> >
>> >         https://github.com/pharo-project/pharo/pull/92
>> >
>> >
>> >         Thanks,
>> >         Guille, Pablo and Rajula
>> >
>> >         --
>> >            [presentation]      Guille Polito
>> >         [CNRS-filaire]         Research Engineer
>> >
>> >
>> >                                Centre de Recherche en Informatique,
>> Signal et
>> >                                Automatique de Lille
>> >                                CRIStAL - UMR 9189
>> >                                French National Center for Scientific
>> Research -
>> >                                http://www.cnrs.fr
>> >
>> >                                Web: http://guillep.github.io
>> >                                Phone: +33 06 52 70 66 13
>> >
>> >
>> >
>> >
>>
>>
>


-- 



Guille Polito

Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille

CRIStAL - UMR 9189

French National Center for Scientific Research - *http://www.cnrs.fr
<http://www.cnrs.fr>*


*Web:* *http://guillep.github.io* <http://guillep.github.io>

*Phone: *+33 06 52 70 66 13
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.pharo.org/pipermail/pharo-dev_lists.pharo.org/attachments/20171012/bb7eb728/attachment-0002.html>


More information about the Pharo-dev mailing list