This is the mail archive of the cygwin-developers mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: native symlink


On Apr 1, 2013, at 12:52 PM, Christopher Faylor <cgf-use-the-mailinglist-please@cygwin.com> wrote:

> 
> If you're referring to this:
> http://sourceware.org/ml/cygwin-developers/2012-12/msg00000.html
> 
> Then I did respond later in the thread.  So, please don't claim that
> I'm irrational or nontechnical.
> 
> To summarize my objection:  It doesn't sound like the native symlink
> can be made to completely emulate a Linux symlink.  That has always
> been the problem with Windows symlinks.


As I said before (and this is discussion of technical detail for programmers), it doesn't need to fully emulate a unix symlink. The design I proposed would attempt a native symlink and then fall back to normal symlink if the operation failed. The posix layer doesn't care which physical form the symlink takes. it works the same either way. The mechanism works nicely...I have prototyped it. 

However, as I said… I'm not going to ask for that model any further as I doubt I could get any traction with and I could live with a lesser capability that is less invasive.




> 
>> What I am lobbying for...
> 
> As Larry indicated, this is not the mailing list for "lobbying" however,
> to save you the trouble of moving to the cygwin mailing list:  As I
> (and Corinna) have said before, I'd rather not complicate the
> labyrinthian path handling code by introducing a new API.  I don't
> really see why one would be needed.

This is why I already throw around words like "irrational."   I have said multiple times that you already read native symlinks. Your "labyrinthian" code already codes does it. Nothing needs to be added to your labyrinthian code for handling paths. But, it seems like this sentence always gets piped right to /dev/null for some reason. That point never gets acknowledged. The same argument just gets repeated over and over despite the fact that it has been answered. To me, that is not rational thinking. sorry.


In fact, let me document for you exactly what I changed in your labyrinthian path handling code…

I added two data members to symlink_info:

size_t mReparsePointType;  //jmg   0 = false. 1 = file reparse point type  2 = directory reparse point type
NTSTATUS mDefaultMethodOpenStatus; //jmg


I then added the following accessor functions:

    bool isReparsePoint(); //jmg
    bool isDirReparsePoint(); //jmg
    NTSTATUS defaultMethodOpenStatus(); //jmg


I modified...
symlink_info::check_reparse_point (HANDLE h, bool remote)

at the end of the function, I added:


    //jmg
    if( (fileattr & FILE_ATTRIBUTE_DIRECTORY) == FILE_ATTRIBUTE_DIRECTORY )
    {
        this->mReparsePointType = 2;
    }
    else
    {
        this->mReparsePointType = 1;
    }


I modified…

int
symlink_info::check (char *path, const suffix_info *suffixes, fs_info &fs,
                     path_conv_handle &conv_hdl)


toward the end, I added..


            status = NtOpenFile (&sym_h, SYNCHRONIZE | GENERIC_READ, &attr, &io,
                                 FILE_SHARE_VALID_FLAGS,
                                 FILE_OPEN_FOR_BACKUP_INTENT
                                 | FILE_SYNCHRONOUS_IO_NONALERT);
            
            if (!NT_SUCCESS (status))
            {
                this->mDefaultMethodOpenStatus = status; //jmg



Basically, the only  modification I did to the existing labyrinthian path handling code was to store some extra data I needed make decisions in my new code higher up in the code stack. That's it!   Now, I hardly think these changes constitute a significant increase in the complexity of the existing path and symlink reading code. 


So what exactly did I change to implement creation of native symlinks (the actual new functionality?  …

 
I modified... 

int
symlink_worker (const char *oldpath, const char *newpath, bool use_winsym,
                unsigned long use_nativesym, bool isdevice) //bool isdevice)



I added a new parameter as you can see. As well as a line to set a local variable:

    bool tmpUseReparsePoints = use_nativesym != 0;
    bool mk_winsym = use_winsym && !tmpUseReparsePoints;


then, I added this section…


    debug_printf ("tmpUseReparsePoints 2 (%d)", tmpUseReparsePoints);
    
    
    if (!isdevice && tmpUseReparsePoints)
    {
        assert(use_nativesym <= 2);  //
        
        bool tmpLinkIsOfDirectoryType = false;
        NTSTATUS tmpNTErrorId = 0;
        int tmpPathConvErrorId = 0;
        size_t tmpResult = attemptReparsePointSymlink(*newpath, *oldpath, tmpNTErrorId, tmpPathConvErrorId, NULL, tmpLinkIsOfDirectoryType); //win32_newpath
        
        debug_printf("result=%d, tmpNTErrorId=%x, tmpPathConvErrorId=%d, tmpLinkIsOfDirectoryType=%d.", tmpResult, tmpNTErrorId, tmpPathConvErrorId, tmpLinkIsOfDirectoryType);
        
        
        if(tmpResult == kSymlinkSuccess)
        {
            res = 0;
            goto done;
        }
        
        if(use_nativesym == 1)
        {
            //weak. if native fails, fall back to default
            //
            //fall through and let the default action happen
            tmpUseReparsePoints = false;
        }
        else  //== 2
        {
            //strong. if reparse point cannot be set, fail.
            //However, if the target doesn't exist, use the default method 
            //come back later and update the link to a reparse point using 'lnmakenative'
            
            if(tmpResult != kSymlinkPathDoesNotExist)
            {
                if (!NT_SUCCESS (tmpNTErrorId))
                {
                    __seterrno_from_nt_status (tmpNTErrorId);
                    goto done;
                }            
                else if(tmpPathConvErrorId != 0)
                {
                    set_errno (tmpPathConvErrorId);
                    goto done;
                }
                else if(tmpResult == kSymlinkPathsTooLong)
                {
                    set_errno (ENAMETOOLONG);
                }
                
                
                //if the error is not an NT function error, and not path conversion error, then it is a win32 error. 
                //fall back to using GetLastError()
                __seterrno();
                goto done;
            }
        }
    }
    
    
    if (mk_winsym)




The rest of the code changes are to add a new environment variable and expose a new function... 

extern "C" int cygwin_update_symlink_to_reparse_point (const char *theLinkPathPtr)


which will update the symlink if it is a default form or leave it alone if it is already native.  Also, there is the actual implementation of attemptReparsePointSymlink() which I can provide if that is useful.



What's my point?   The new functionality I've added…namely…creating native symlinks is entirely encapsulated in one small area of code.  It does not add anything significant to the labyrinth of which you are concerned. The only thing I added to the labyrinth was a couple of extra member variables to persist data I needed later in the process. Your existing labyrinth is pretty much sufficient as it is!


All of that said, I'm no longer asking for the changes to symlink_worker. A person who has my needs can function with just cygwin_update_symlink_to_reparse_point() and the lnmakenative utility I wrote. Certainly a new function..

extern "C" int
symlink_native (const char *oldpath, const char *newpath)

would be nice,  but I won't push my luck if I can get the bare minimum.





> However, if you think it's a great idea to have a utility which does
> stuff to and with native symlinks then that's something that you could
> write yourself and propose to be included in the Cygwin distribution.
> That would be something you could discuss in the cygwin mailing list
> and, ultimately, the cygwin-apps mailing list.


I did write the utility myself….as I have said repeatedly. It is in production use at my company.  But, more is desirable than just an app. Your system is designed so that so that all the logic dealing with paths and symlinks is in path.cc. That is where the internal structure of the cygwin symlink is encapsulated. It is most appropriate for the core logic for such a feature to go there as that fits into the existing design model.  Furthermore, I already have the code written for you. You may wish to repackage it a bit to suit your tastes and make sure there are no flaws in my design (I make no claim to be an expert on the internals of cygwin), but the core logic is done and it works.


I'm certainly willing to join this other mailing list and make a formal proposal. But, I don't want to waste my time if the programmers don't want to be bothered. So the question is… have I made my case sufficiently so that the programmers will be willing to work with me?


-James















Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]