Re: [PATCH] Support DOS paths in dash
Edward Lam edw...@sidefx.com wrote: On Oct 1, 2014, at 8:41 PM, Herbert Xu herb...@gondor.apana.org.au wrote: Eric Blake ebl...@redhat.com wrote: I'm not interested in burdening the cygwin build of dash with a one-off patch, so I'd like to gauge the upstream thoughts - is it worth including platform-specific patches like this (no penalty to build size of non-cygwin platforms, and on cygwin, it allows 'cd c:/' to behave as shorthand for 'cd /cygdrive/c/')? If the patch lands in dash.git, then I'll rebuild the cygwin port of dash to include a backport (rather than waiting for 0.5.9 to be released). If there is no interest, I'd rather just drop the patch. The cygwin community already states that /cygdrive/c notation is the official way to access drive letters, and that if 'c:/' works it is nice, but it is not a design goal to always have it work. It's fine by me. Is that a yes to allow platform-specific patches into dash? If there is zero impact on other platforms and it doesn't adversely impact the maintainability then yes. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe dash in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Support DOS paths in dash
Eric Blake ebl...@redhat.com wrote: I'm not interested in burdening the cygwin build of dash with a one-off patch, so I'd like to gauge the upstream thoughts - is it worth including platform-specific patches like this (no penalty to build size of non-cygwin platforms, and on cygwin, it allows 'cd c:/' to behave as shorthand for 'cd /cygdrive/c/')? If the patch lands in dash.git, then I'll rebuild the cygwin port of dash to include a backport (rather than waiting for 0.5.9 to be released). If there is no interest, I'd rather just drop the patch. The cygwin community already states that /cygdrive/c notation is the official way to access drive letters, and that if 'c:/' works it is nice, but it is not a design goal to always have it work. It's fine by me. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe dash in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Support DOS paths in dash
On Oct 1, 2014, at 8:41 PM, Herbert Xu herb...@gondor.apana.org.au wrote: Eric Blake ebl...@redhat.com wrote: I'm not interested in burdening the cygwin build of dash with a one-off patch, so I'd like to gauge the upstream thoughts - is it worth including platform-specific patches like this (no penalty to build size of non-cygwin platforms, and on cygwin, it allows 'cd c:/' to behave as shorthand for 'cd /cygdrive/c/')? If the patch lands in dash.git, then I'll rebuild the cygwin port of dash to include a backport (rather than waiting for 0.5.9 to be released). If there is no interest, I'd rather just drop the patch. The cygwin community already states that /cygdrive/c notation is the official way to access drive letters, and that if 'c:/' works it is nice, but it is not a design goal to always have it work. It's fine by me. Is that a yes to allow platform-specific patches into dash? Thanks, -Edward -- To unsubscribe from this list: send the line unsubscribe dash in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Support DOS paths in dash
[I noticed an old thread in my inbox while packaging dash 0.5.8 for Cygwin] On 03/28/2013 09:08 AM, Edward Lam wrote [to the cygwin list]: The problem is that dash tries to convert c:/windows to an absolute path, since it doesn't start with /. I suppose I could teach dash to recognize [letter]:/ as absolute paths, although that makes dash larger, and puts a burden on me (since I can guarantee upstream dash won't accept such a patch). I just don't care enough for DOS paths so I won't fix. Me neither. And since you can use /cygdrive/c, not c:/, I won't bother to fix it. Hi Folks, I finally got down to looking at how to fix this in dash and came up with the attached patch (against dash-0.5.7). It's simple enough and so cd now works. Please consider this for Cygwin. I'm not interested in burdening the cygwin build of dash with a one-off patch, so I'd like to gauge the upstream thoughts - is it worth including platform-specific patches like this (no penalty to build size of non-cygwin platforms, and on cygwin, it allows 'cd c:/' to behave as shorthand for 'cd /cygdrive/c/')? If the patch lands in dash.git, then I'll rebuild the cygwin port of dash to include a backport (rather than waiting for 0.5.9 to be released). If there is no interest, I'd rather just drop the patch. The cygwin community already states that /cygdrive/c notation is the official way to access drive letters, and that if 'c:/' works it is nice, but it is not a design goal to always have it work. --- src/cd.c 2011-03-15 03:18:06.0 -0400 +++ src/cd.new.c 2013-03-28 11:03:32.649576500 -0400 @@ -38,6 +38,9 @@ #include string.h #include unistd.h #include limits.h +#ifdef __CYGWIN__ +#include sys/cygwin.h +#endif /* * The cd and pwd commands. @@ -194,6 +197,11 @@ char *cdcomppath; const char *lim; +#ifdef __CYGWIN__ +char pathbuf[PATH_MAX + 1]; +cygwin_conv_to_full_posix_path (dir, pathbuf); By the way, cygwin_conv_to_full_posix_path() is deprecated (it suffers from possible buffer overflow); these days, it's preferred to use: cygwin_conv_path (CCP_WIN_A_TO_POSIX | CCP_RELATIVE, string, pathbuf, sizeof(pathbuf)) So, if there is interest in this patch upstream, I can respin it. + dir = pathbuf; +#endif cdcomppath = sstrdup(dir); STARTSTACKSTR(new); if (*dir != '/') { -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature