#453 Fix order of msys_symlink arguments

[Ladislav Michl]

According to POSIX standard symlink name is second argument
http://www.opengroup.org/onlinepubs/009695399/functions/symlink.html

No functional changes otherwise, except memory is no longer leaked
when creating symlink to directory.

[Ladislav Michl]

[Keith Marshall]

Thanks for the patch. I'll leave it for Cesar, to conduct a detailed review, but just one comment, on cursory inspection:

The semantics of copying and linking are quite different; while it is quite correct that the POSIX spec for the symlink() function places the 'topath' argument first, followed by the 'frompath', (i.e. we follow the reference from the created link to the target data), is it really imperative that we maintain this relationship within what is purely an internal implementation detail? IOW, while it may make sense to reverse the argument order to msys_symlink(), it perhaps isn't essential. The symlink() function, as implemented in path.cc already appears to provide a POSIXly correct API; that the argument order appears to be reversed when calling the msys_symlink() helper may be a potential cause of confusion, but it is an internal detail, which isn't really problematic provided it is applied consistently at every point of use.

OTOH, the reversal of the order of the first two arguments to RecursiveCopy, while also an internal detail, just seems semantically wrong. At a shell prompt, we express the command to create a symbolic link as:

ln -s topath frompath

with the argument order semantically matching the order of arguments to be passed to the symlink() function. However, to perform a recursive copy, we express the command as:

cp -r frompath topath

Note the semantic difference. In both cases, the final argument represents what is to be created, and the preceding argument specifies whence the data is to be sourced, but in the case of the copy operation, we copy the data from the source to the destination, whereas in the case of linking, we refer from the *destination* back to the source.

[Ladislav Michl]

Well, the patch was ment as a preparation for native symlink merge.
In the long term plan, I'd like to rewrite path.cc without all cygwin
leftovers and let symlink function exist only in symlink.cc. Therefore
I swapped argument of msys_symlink function which is now
'internal implementation', but as symlink function only call this one,
I'd like to remove it and rename msys_symlink to symlink
(assuming cygwin fork happened only once, and there are no plans
to merge with 'upstream')

As far as RecursiveCopy goes, I did it just for convenience, to
make it consistent with msys_symlink and I do agree, that
these arguments would be better named src and dst.
Shall I update patch this way?

[Earnie Boyd]

As it stands I don't know that there is any reason to think of any plans for merging upstream. More likely merging upstream to the fork may happen, at least that is the reason for the msys_symlink that and easily recognizing which code went with the fork. I'll let Cesar decide if the functionality should be merged back to symlink.

[Ladislav Michl]

[Ladislav Michl]

Uploaded another version of patch with more meaningfull argument
names.

[Cesar Strauss]

Looks good, thanks. Could you provide an entry for Changelog.MSYS as well?

Thanks,
Cesar

[Ladislav Michl]

While writing Changelog entry, I noticed that the last one is not sorted by time.
In case it is a mistake, changelog entry not provided as a path to avoid
patching problem on corrected file.

CHANGELOG
2010.08.20 Ladislav Michl <ladis@users.sourceforge.net>

\* msys\_symlink.cc \(msys\_symlink\): Swap argument order to match POSIX.
\* path.cc \(symlink\): Swap msys\_symlink arguments to match msys\_symlink.

[Keith Marshall]

> ... changelog entry not provided as a path to avoid
> patching problem on corrected file.

A ChangeLog entry should NEVER be provided in patch format anyway. It may be provided as a text block at the head of a patch file, but should not have patch headers associated with it, (as the actual patch content will have). The maintainer applying and committing the patch will simply copy and paste the ChangeLog block directly into the top of the ChangeLog file, and adjust the date stamp accordingly, at time of committal.

Rationale: with parallel development, there may be many patches applied between the time you create your patch, and the time it gets committed. Each of these will have its own ChangeLog entry, so if you've provided yours as a patch, it will not match the state of the ChangeLog file at time of committal, and will fail.

[Cesar Strauss]

Applied, thanks!

> While writing Changelog entry, I noticed that the last one
> is not sorted by time.

It's intentional. I merged an old change from Cygwin and choose to preserve the author and date info. Note the indentation of this entry.

Thanks,
Cesar