Loading ...
Sorry, an error occurred while loading the content.

vim patch for cygwin

Expand Messages
  • Luca Masini
    I m not sure if this is the right way to submit a patch. Anyway the attached patch should solve the problem with symbolic links. For ex. when editing
    Message 1 of 9 , Apr 15, 2007
    • 0 Attachment
      I'm not sure if this is the right way to submit a patch.

      Anyway the attached patch should solve the problem with symbolic links.
      For ex. when editing /etc/hosts we get the warning
      E303: Unable to open swap file for "hosts", recovery impossible
      (/etc/hosts is a symlink in cygwin)

      Regards.

      Luca.
    • Bram Moolenaar
      ... The mch_FullName() in os_unix.c already takes care of symlinks. Why would this extra code for Cygwin be needed? If something needs to be patched it s
      Message 2 of 9 , Apr 16, 2007
      • 0 Attachment
        Luca Masini wrote:

        > I'm not sure if this is the right way to submit a patch.
        >
        > Anyway the attached patch should solve the problem with symbolic links.
        > For ex. when editing /etc/hosts we get the warning
        > E303: Unable to open swap file for "hosts", recovery impossible
        > (/etc/hosts is a symlink in cygwin)

        The mch_FullName() in os_unix.c already takes care of symlinks. Why
        would this extra code for Cygwin be needed?

        If something needs to be patched it's probably best done in mch_FullName().

        --
        If you don't get everything you want, think of
        everything you didn't get and don't want.

        /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
        /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
        \\\ download, build and distribute -- http://www.A-A-P.org ///
        \\\ help me help AIDS victims -- http://ICCF-Holland.org ///
      • Bram Moolenaar
        ... A symlink from a Unix-style path to a DOS-style path? That appears to be the problem to me. It s a bad symlink. Unix programs will not work properly
        Message 3 of 9 , Apr 16, 2007
        • 0 Attachment
          Luca Masini wrote:

          > Bram Moolenaar wrote:
          >
          > > The mch_FullName() in os_unix.c already takes care of symlinks. Why
          > > would this extra code for Cygwin be needed?
          > >
          > > If something needs to be patched it's probably best done in
          > mch_FullName().
          >
          >
          > In cygwin /etc/hosts is a symbolic link to
          > C:\WINDOWS\system32\drivers\etc\hosts
          > and if expanded in that way is not recognized as a full path because
          > does not start with / or with ~.

          A symlink from a Unix-style path to a DOS-style path? That appears to
          be the problem to me. It's a bad symlink. Unix programs will not work
          properly with this symlink.

          > The small patch change it in posix format
          > /cygdrive/c/WINDOWS/system32/drivers/etc/hosts
          > so it is recognized as a full path

          Then why don't you change the symlink to this path?

          > and the .swp file is created (in the other case there is the message
          > "*E303* Unable to open swap file")
          >
          > Maybe the mch_FullName() is a better place to put the
          > cygwin_conv_to_posix_path().
          > I have put it there because there is also slash_adjust() for other
          > platform.

          slash_adjust() only changes "\" to "/", which doesn't change the length
          of the path. I assume that cygwin_conv_to_posix_path() can change the
          length, thus it's a very different thing.

          I think the Cygwin version of Vim should never see "C:/path" things,
          thus converting all paths sounds like a good idea. Probably near the
          start of mch_FullName(). However, I don't know if this breaks
          something. I never use the Cygwin version.

          --
          DENNIS: Oh, very nice. King, eh! I expect you've got a palace and fine
          clothes and courtiers and plenty of food. And how d'you get that? By
          exploiting the workers! By hanging on to outdated imperialist dogma
          which perpetuates the social and economic differences in our society!
          "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

          /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
          /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
          \\\ download, build and distribute -- http://www.A-A-P.org ///
          \\\ help me help AIDS victims -- http://ICCF-Holland.org ///
        • Luca Masini
          Luca Masini & Bram Moolenaar wrote: ... This is common in cygwin. It supports different path formats. ... I come across the problem because I
          Message 4 of 9 , Apr 16, 2007
          • 0 Attachment
            Luca Masini & Bram Moolenaar wrote:

            <snip>

            >> In cygwin /etc/hosts is a symbolic link to
            >> C:\WINDOWS\system32\drivers\etc\hosts
            >> and if expanded in that way is not recognized as a full path because
            >> does not start with / or with ~.
            >
            > A symlink from a Unix-style path to a DOS-style path? That appears to
            > be the problem to me. It's a bad symlink. Unix programs will not work
            > properly with this symlink.

            This is common in cygwin.
            It supports different path formats.

            <snip>

            > Then why don't you change the symlink to this path?

            I come across the problem because I often need to edit /etc/hosts
            and that is a predefined symbolic link.
            But a user can define other symbolic link as he or she likes.
            The symbolic link problem is general.

            <snip>

            > slash_adjust() only changes "\" to "/", which doesn't change the length
            > of the path. I assume that cygwin_conv_to_posix_path() can change the
            > length, thus it's a very different thing.

            if I remember correctly the size of buf was MAXPATHL o something similar
            so the change of lenght should not be a problem in this context...
            but I agree that if the function is called with len less than MAXPATHL
            this will be a problem.

            > I think the Cygwin version of Vim should never see "C:/path" things,
            > thus converting all paths sounds like a good idea. Probably near the
            > start of mch_FullName(). However, I don't know if this breaks
            > something. I never use the Cygwin version.

            The used function is explained here
            http://www.cygwin.com/cygwin-api/func-cygwin-conv-to-posix-path.html

            My idea was to convert to posix the path as soon as possible
            (just after the resolution of the symlink) with a snippet like this
            #if defined(__CYGWIN__)
            cygwin_conv_to_posix_path( buf, buf );
            #endif
            Then the path is posix and the problem of having a mix of posix
            and windows path is no more present.

            It is late here....
            I will look tomorrow at the mch_FullName() in os_unix.c

            Luca.

            >
            >
          • Luca Masini
            Luca Masini wrote: ... The patch moved in mch_FullName() in os_unix.c
            Message 5 of 9 , Apr 16, 2007
            • 0 Attachment
              Luca Masini wrote:

              <snip>
              > It is late here....
              > I will look tomorrow at the mch_FullName() in os_unix.c
              The patch moved in mch_FullName() in os_unix.c
            • Bram Moolenaar
              ... Why at the end of mch_FullName()? I would guess it needs to be done before this line: /* expand it if forced or not an absolute path */ if (force ||
              Message 6 of 9 , Apr 17, 2007
              • 0 Attachment
                Luca Masini wrote:

                > <snip>
                > > It is late here....
                > > I will look tomorrow at the mch_FullName() in os_unix.c
                > The patch moved in mch_FullName() in os_unix.c
                >
                >
                > --------------030608030905060601020801
                > Content-Type: text/x-patch;
                > name="vim.patch"
                > Content-Transfer-Encoding: 7bit
                > Content-Disposition: inline;
                > filename="vim.patch"
                >
                > diff --recursive --unified vim70/src/os_unix.c vim70-patched/src/os_unix.c
                > --- vim70/src/os_unix.c 2006-05-01 10:13:15.000000000 +0200
                > +++ vim70-patched/src/os_unix.c 2007-04-16 22:57:06.980184000 +0200
                > @@ -2350,6 +2350,10 @@
                > if (STRCMP(fname, ".") != 0)
                > STRCAT(buf, fname);
                >
                > +#if defined(__CYGWIN__)
                > + cygwin_conv_to_posix_path( buf, buf );
                > +#endif
                > +
                > return OK;
                > }

                Why at the end of mch_FullName()? I would guess it needs to be done
                before this line:

                /* expand it if forced or not an absolute path */
                if (force || !mch_isFullName(fname))

                Then all what follows works on the Unix path instead of the MS-DOS path,
                which probably won't work.

                Also read the comments in mch_FullName() to get an idea of what is going
                on there.

                --
                Mrs Abbott: I'm a paediatrician.
                Basil: Feet?
                Mrs Abbott: Children.
                Sybil: Oh, Basil!
                Basil: Well, children have feet, don't they? That's how they move
                around, my dear. You must take a look next time, it's most
                interesting. (Fawlty Towers)

                /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
                /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
                \\\ download, build and distribute -- http://www.A-A-P.org ///
                \\\ help me help AIDS victims -- http://ICCF-Holland.org ///
              • Luca Masini
                ... Ok. Luca.
                Message 7 of 9 , Apr 17, 2007
                • 0 Attachment
                  Bram Moolenaar wrote:

                  > Why at the end of mch_FullName()? I would guess it needs to be done
                  > before this line:
                  >
                  > /* expand it if forced or not an absolute path */
                  > if (force || !mch_isFullName(fname))

                  Ok.

                  Luca.
                • Bram Moolenaar
                  ... Hmm, I m not getting the feeling this has been properly tested. I ll await comments for a while. In src/main.c I found this, perhaps it s also needed in
                  Message 8 of 9 , Apr 18, 2007
                  • 0 Attachment
                    Luca Masini wrote:

                    > Bram Moolenaar wrote:
                    >
                    > > Why at the end of mch_FullName()? I would guess it needs to be done
                    > > before this line:
                    > >
                    > > /* expand it if forced or not an absolute path */
                    > > if (force || !mch_isFullName(fname))
                    >
                    > Ok.
                    >
                    > Luca.
                    >
                    > diff --recursive --unified vim70/src/os_unix.c vim70-patch/src/os_unix.c
                    > --- vim70/src/os_unix.c 2006-05-01 10:13:15.000000000 +0200
                    > +++ vim70-patch/src/os_unix.c 2007-04-18 00:40:52.747808000 +0200
                    > @@ -2228,6 +2228,10 @@
                    > fname = vms_fixfilename(fname);
                    > #endif
                    >
                    > +#ifdef __CYGWIN__
                    > + cygwin_conv_to_posix_path( fname, fname );
                    > +#endif
                    > +
                    > /* expand it if forced or not an absolute path */
                    > if (force || !mch_isFullName(fname))
                    > {


                    Hmm, I'm not getting the feeling this has been properly tested. I'll
                    await comments for a while.

                    In src/main.c I found this, perhaps it's also needed in os_unix.c:

                    #ifdef __CYGWIN__
                    # ifndef WIN32
                    # include <sys/cygwin.h> /* for cygwin_conv_to_posix_path() */
                    # endif
                    # include <limits.h>
                    #endif


                    --
                    FIRST VILLAGER: We have found a witch. May we burn her?
                    "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

                    /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
                    /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
                    \\\ download, build and distribute -- http://www.A-A-P.org ///
                    \\\ help me help AIDS victims -- http://ICCF-Holland.org ///
                  • Luca Masini
                    ... I did t run any regression test. The only problem on cygwin is the E303: Unable to open swap file... message. Looking at vim under the ddd/gdb debugger I
                    Message 9 of 9 , Apr 18, 2007
                    • 0 Attachment
                      Bram Moolenaar wrote:

                      > Hmm, I'm not getting the feeling this has been properly tested. I'll
                      > await comments for a while.

                      I did't run any regression test.

                      The only problem on cygwin is the "E303: Unable to open swap file..."
                      message.

                      Looking at vim under the ddd/gdb debugger I found that it
                      was trying to compose the name of the .swp file concatenating
                      a posix path with a windows one (that was the result of the
                      symlink exansion not recognized as an absolute path) so

                      I patched with cygwin_conv_to_posix_path() and recompiled.

                      I'm using this version now (just changed the position of the patch
                      as you suggested) and it works here.

                      > In src/main.c I found this, perhaps it's also needed in os_unix.c:
                      >
                      > #ifdef __CYGWIN__
                      > # ifndef WIN32
                      > # include <sys/cygwin.h> /* for cygwin_conv_to_posix_path() */
                      > # endif
                      > # include <limits.h>
                      > #endif

                      It is not strictly needed because gcc compiled without having this snippet.
                      Maybe having __CYGWIN__ defined cause the inclusion of sys/cygwin.h
                      in other place.

                      But from wath I see that can be useful if you compile from cygwin
                      to windows native and you want to avoid the inclusion of
                      cygwin_conv_to_posix_path() when WIN32 is defined.

                      Maybe the better solution is to avoid mixing different
                      kind of path instead of patching a symlink that was expanded
                      as a windows path when it should had be expanded as a posix path.

                      (my current solution).
                      ;-)


                      Luca.
                    Your message has been successfully submitted and would be delivered to recipients shortly.