Checkout Tools
  • last updated 1 hour ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
Win32: fix an incorrect error status being propagated to the caller in case

where we fail to stat the destination path while being in the middle of a

failed rename.

So, if we fail to stat the destination in the middle of such rename, propagate

the *original* error. Because overriding the original error with the new one

loses information about the actual cause of failure and may even confuse some

of the callers, who for instance attempt to recover in case of an EACCES,

since they may not be receiving that error at all.

(This is the behavior we had for a long time, even before r1865518, but now

seems to be an appropriate moment to fix that)

* subversion/libsvn_subr/io.c

(win32_file_rename): Use a separate `stat_err` for the case of the failed

GetFileAttributes() call. If we failed to stat the file, return the

original `err` status to the caller.

* subversion/libsvn_subr/io.c

(win32_file_rename): Rename `status` to `err`. This lays the groundwork for

fixing the incorrect error status being propagated to the caller in case

where we fail to stat the destination path while being in the middle of

a failed rename.

Win32: prevent svn_io_file_rename2() from spinning in the retry loop in a

racy case where the rename function clears the read-only attribute from the

destination file, but another thread or process is quick enough to set it

back before the original rename function has a chance to proceed.

Despite sounding like an uncommon case, this may happen when the API is

being used to atomically update the file contents while also altering its

read-only attribute (and where such updates can occur in parallel).

Fix this by including the call that clears the read-only attribute on the

destination into the retry loop instead of calling it only once before

looping.

* subversion/libsvn_subr/io.c

(win32_file_rename): Handle EACCES/EEXIST and attempt to clear the

destination read-only attribute if it's there.

(svn_io_file_rename2): Don't call svn_io_set_file_read_write() in the

Win32-specific portion of this function, since clearing the read-only

attribute is now a part of the win32_file_rename() function.

* subversion/libsvn_subr/io.c

(io_win_check_path): Tweak comment, no functional changes.

io: Implement the functions that check for node existence using the native

Win32 APIs on Windows:

svn_io_check_path()

svn_io_check_resolved_path()

svn_io_check_special_path()

This changeset aims for two distinct things:

1) First of all, starting from r1833621, these functions were patched to stop

checking symlinks on Windows. The root cause for this change has been an

incorrect implementation of stat in APR that didn't properly distinguish

between various types of reparse points, some of which are not symlinks.

Providing the custom implementation allows us to remove the hack and

properly handle such reparse points irrespectively of the APR version

in use.

Additional details on the core of the issue can be found here:

- https://github.com/golang/go/issues/23684

- https://github.com/dotnet/corefx/issues/24250

2) These APIs are used in various performance-critical code paths such as

in the core part of `svn st`.

On Win32 the necessary answers for these particular functions can be

obtained with a single call to GetFileAttributes(), which is much faster

than using the generic stat implementations from APR 1.6.x and 1.7.x

(I believe that it would be even slower in the latter case).

A couple of quick tests show about 20%-25% improvement in the speed of

`svn st` for a large working copy. The improvement may be more significant

with indexers or virus scanners, as just asking for file attributes may

completely avoid opening a file (by retrieving the result through the

fast I/O QueryOpen).

* subversion/libsvn_subr/io.c

(FILE_ATTRIBUTE_TAG_INFO, FileAttributeTagInfo): Add these definitions

for old versions of Windows SDK.

(typedef GetFileInformationByHandleEx_t,

get_file_information_by_handle_ex_proc): New.

(win_init_dynamic_imports): Import `GetFileInformationByHandleEx()`.

(win32_get_file_information_by_handle): New helper function.

(io_win_check_path): New helper with the Win32 implementation required

for the "check path" functions.

(svn_io_check_path,

svn_io_check_resolved_path,

svn_io_check_special_path): Invoke the new helper.

(io_check_path): Undo the workaround from r1833621 that stopped passing

APR_FINFO_LINK when performing a stat.

Follow up to r1854072: Reduce the number of file-open operations on Windows.

* subversion/libsvn_subr/io.c (svn_io_dir_remove_nonrecursive):

On Windows, only remove the read-only flag from the directory after

the initial deletion failed.

Patch by: kotkov

Follow up to r1854072: Fix a typo in Windows-specific code.

* subversion/libsvn_subr/io.c (io_set_readonly_flag): Fix function signature.

Fix issue #4806: Remove on-disk trees with read-only directories in them.

* subversion/libsvn_subr/io.c

(io_set_perms): New; helper function for io_set_*_perms.

(io_set_file_perms): Use io_set_perms.

(io_set_dir_perms): New; like io_set_file_perms, but for directories.

(io_set_readonly_flag): New; helper function for setting the read-only flag.

(svn_io_set_file_read_only,

svn_io_set_file_read_write): Use io_set_readonly_flag.

(svn_io_remove_dir2): On Unix, make the parent directory writable before

trying to remove its children.

(svn_io_dir_remove_nonrecursive): On Windows, remove a directory's

read-only flag before trying to remove the directory.

* subversion/tests/libsvn_subr/io-test.c

(create_dir_tree): New helper function.

(test_rmtree_all_writable,

test_rmtree_file_readonly,

test_rmtree_dir_readonly,

test_rmtree_all_readonly): New test cases.

(test_funcs): Activate the new test cases.

  1. … 1 more file in changeset.
Following up on r1836306, provide an alternative fix for allowing things

like "svn commit -F <(echo foo)" that doesn't require a more expensive

apr_file_info_get(APR_FINFO_SIZE | APR_FINFO_TYPE) syscall in the

low-level and commonly used function.

Instead of additionally blacklisting APR_PIPE handles, rework the code to

work under the conditions where `finfo.size` may be wrong. That is, try to

read one more byte than necessary, expect to see an EOF, but if we don't

(e.g., for pipes), fall through to reading the remaining part of the file

chunk-by-chunk.

This approach should work correctly under all possible cases where `finfo.

size` is incorrect, and also allows to just ask for APR_FINFO_SIZE, which

is generally cheaper. At least on Windows, this uses GetFileSizeEx() and

results in a single syscall.

* subversion/libsvn_subr/io.c

(stringbuf_from_aprfile): Replace the additional blacklist condition for

APR_PIPEs with the approach described above.

* subversion/libsvn_subr/io.c

(stringbuf_from_aprfile): If the file is a FIFO then do not rely on

the filesize as it will always be zero. This allows things like:

svn commit -F <(echo foo)

svnmucc propsetf p <(echo bar) URL

* subversion/libsvn_subr/io.c

(svn_io_dir_walk2): Correct an old comment.

Since on Windows Subversion does not handle symlinks, never check for reparse points.

* subversion/libsvn_subr/io.c

(io_check_path): ignore the 'resolve_symlinks' flag on Windows via #ifdef

Update issue tracker links in comments, from Tigris (issuezilla) to Apache (Jira).

URL fragment identifiers like '#desc5' are left in place, not yet updated.

This is a merge of r1828508 from the 'shelve-checkpoint' branch where I

committed it by mistake.

  1. … 43 more files in changeset.
Rename and move the new-for-1.10 function 'svn_io_stdin_readline()' to

'svn_cmdline__stdin_readline()'.

It is just a wrapper around existing API functions and lacks some checking and

options so was deemed not suitable to go into the public API in its current

form.

* subversion/include/private/svn_cmdline_private.h,

subversion/libsvn_subr/cmdline.c

(svn_cmdline__stdin_readline): Rename and move to here...

* subversion/include/svn_io.h

* subversion/libsvn_subr/io.c

(svn_io_stdin_readline): ... from here.

* subversion/svnbench/svnbench.c,

subversion/svnmucc/svnmucc.c,

subversion/svnrdump/svnrdump.c,

subversion/svn/svn.c,

tools/client-side/svnconflict/svnconflict.c,

tools/client-side/svn-mergeinfo-normalizer/svn-mergeinfo-normalizer.c,

tools/dev/svnmover/svnmover.c

(sub_main): Track the rename.

  1. … 10 more files in changeset.
Introduce a new global option: --password-from-stdin

This new option makes Subversion's command line applications read a password

from standard input. It can be used as an alternative to the --password option

in order to provide passwords without leaking them to argv, which on Unix-like

systems can be viewed by anyone with tools such as ps(1).

Patch by: William Orr <will@worrbase.com>

* subversion/include/svn_io.h

(svn_io_stdin_readline): Declare.

* subversion/libsvn_subr/io.c

(svn_io_stdin_readline): New public API which reads a line of input from

standard input.

* subversion/svn/cl.h

(svn_cl__opt_state_t): Declare auth_password_from_stdin option.

* subversion/svn/svn.c,

subversion/svnbench/svnbench.c

(svn_cl__longopt_t): Add opt_auth_password_from_stdin.

(svn_cl__options): Add --password-from-stdin.

(svn_cl__global_options): Add opt_auth_password_from_stdin.

(sub_main): Handle the new option.

* subversion/svnmucc/svnmucc.c

(help): Add --password-from-stdin option to help text.

(sub_main): Handle the new option.

* subversion/svnrdump/svnrdump.c

(svn_svnrdump__longopt_t): Add opt_auth_password_from_stdin.

(svnrdump__options, opt_baton_t): Add --password-from-stdin and --dumpfile.

(opt_baton_t, replay_revisions, dump_cmp, load_cmd, sub_main): Add support

for the new options. Since svnrdump has historically been reading dump file

data from stdin the --password-from-stdin option requires --dumpfile.

* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout:

Adjust exected output.

* tools/client-side/svn-mergeinfo-normalizer/svn-mergeinfo-normalizer.c

(svn_min__longopt_t, svn_min__options): Add opt_auth_password_from_stdin.

(svn_min__options): Add --password-from-stdin.

(sub_main): Support the new options.

* tools/client-side/svnconflict/svnconflict.c

(svnconflict_cmd_baton_t, global_options): Add opt_auth_password_from_stdin.

(svnconflict_options): Add --password-from-stdin.

(sub_main): Support the new options.

  1. … 10 more files in changeset.
Fix a potential bug and an API contract violation in the Win32 implementation

of svn_io_file_write_full().

Even in the case when a specific workaround for issue #1789 is not required,

the implementation of this function has been calling apr_file_write() that

can do a short write and return APR_SUCCESS. If that happens, the

guarantee of the "write full" function would not be fulfilled in the sense

that the function would write less bytes than requested without an

associated error, and that could cause issues further down the line.

On Windows, apr_file_write() is implemented as a call to WriteFile() that

shouldn't result in short writes for file handles, but such short writes

are still possible, for example, for pipes. That is where the previous

implementation could be working improperly. (See the "Remarks" section

in https://msdn.microsoft.com/en-us/library/windows/desktop/aa365747).

* subversion/libsvn_subr/io.c

(svn_io_file_write_full): Always attempt a full write in the Win32-specific

code path. Join and tweak the comments on issue #1789, add the issue

number and place the comment right before the code that implements a

workaround.

* subversion/libsvn_subr/io.c

(svn_io_file_trunc): Fix typo in a comment.

Resolve two minor Windows specific corectness warnings found by a newer

compiler version.

* subversion/libsvn_subr/io.c

(svn_io__utf8_to_unicode_longpath): Remove unnecessary truncate.

* subversion/libsvn_subr/win32_crashrpt.c

(write_value): Properly print pointersized value.

  1. … 1 more file in changeset.
Add a workaround for yet another issue with APR's apr_file_trunc.

The previous workaround is ineffective if the last file access had been

a read. Now, we force it into to "write mode" internally to have the

existing workaround kick in.

Luckily, this only affects 'svnadmin pack' for FSFS format 7 and FSX.

The other functions using trunc should have no problem with the added

overhead.

* subversion/libsvn_subr/io.c

(svn_io_file_trunc): Admend the existing workaround with a dummy-write.

* subversion/tests/libsvn_subr/io-test.c

(test_apr_trunc_workaround): New test demonstrating the problem.

(test_funcs): Register the new test.

  1. … 1 more file in changeset.
Follow-up to r1755486: Rename svn_stream_checksum() to

svn_stream_contents_checksum().

Suggested by: danielsh

* subversion/include/svn_io.h

* subversion/libsvn_subr/stream.c

(svn_stream_checksum): Rename to svn_stream_contents_checksum().

(compute_stream_checksum): Update docstring.

* subversion/libsvn_fs/fs-loader.c

* subversion/libsvn_subr/io.c

* subversion/tests/libsvn_fs/fs-test.c

* subversion/tests/libsvn_subr/stream-test.c

(svn_fs_file_checksum, svn_io_file_checksum2, get_file_checksum,

test_stream_checksum): Adapt callers.

  1. … 5 more files in changeset.
Use Unicode Windows API.

* subversion/libsvn_subr/io.c

(win_init_dynamic_imports): Use GetModuleHandleW instead of GetModuleHandleA.

* subversion/libsvn_subr/sysinfo.c

(system_info): Use GetModuleHandleW instead of GetModuleHandleA.

(enum_loaded_modules): Use GetModuleHandleW/LoadLibraryW instead of

GetModuleHandleA/LoadLibraryA.

  1. … 1 more file in changeset.
Remove redundant check.

* subversion/libsvn_subr/io.c

(svn_io_remove_file2): Remove check for !apr_err, because we have the same

check few lines below.

Introduce svn_stream_checksum() function to calculate checksum of specified

stream contents. Use new API where it makes sense.

* subversion/include/svn_io.h

(svn_stream_checksum): New.

* subversion/libsvn_subr/stream.c

(compute_stream_checksum): New. Helper for svn_stream_checksum().

(svn_stream_checksum): New.

* subversion/tests/libsvn_subr/stream-test.c

(test_stream_checksum): New. Simple test for svn_stream_checksum().

(test_funcs): Add test_stream_checksum to test list.

* subversion/libsvn_fs/fs-loader.c

* subversion/libsvn_repos/config_pool.c

* subversion/libsvn_subr/io.c

* subversion/tests/libsvn_fs/fs-test.c

(svn_fs_file_checksum, auto_parse, svn_io_file_checksum2,

get_file_checksum): Use svn_stream_checksum() instead of

svn_stream_checksummed2(READ_ALL=TRUE) + svn_stream_close().

  1. … 6 more files in changeset.
Fix a minor inefficiency when generating a "random" file name.

* subversion/libsvn_subr/io.c

(get_default_file_perms): Use more of the entropy that we are given.

Use svn_io_file_get_offset() instead of svn_io_file_seek(APR_CUR) where it

makes sense.

* subversion/libsvn_client/patch.c

* subversion/libsvn_diff/parse-diff.c

* subversion/libsvn_repos/dump.c

* subversion/libsvn_subr/io.c

* subversion/libsvn_subr/stream.c

* subversion/tests/libsvn_subr/io-test.c

(tell_file, read_handler_base85, hunk_readline_original_or_modified,

svn_diff_hunk_readline_diff_text, parse_next_hunk, parse_binary_patch,

svn_diff_parse_next_patch, store_delta, svn_io_file_readline,

mark_handler_apr, test_file_readline, aligned_seek): Use

svn_io_file_get_offset() instead of svn_io_file_seek(0, APR_CUR).

  1. … 5 more files in changeset.
Promote libsvn_fs_fs private helper svn_fs_fs__get_file_offset() to public

libsvn_subr function svn_io_file_get_offset().

* subversion/libsvn_fs_fs/util.c

* subversion/libsvn_fs_fs/util.h

(svn_fs_fs__get_file_offset): Move/rename to ...

* subversion/include/svn_io.h

* subversion/libsvn_subr/io.c

(svn_io_file_get_offset): ... here.

* subversion/libsvn_fs_fs/cached_data.c

* subversion/libsvn_fs_fs/index.c

* subversion/libsvn_fs_fs/pack.c

* subversion/libsvn_fs_fs/transaction.c

* subversion/libsvn_fs_fs/verify.c

(get_file_offset, stream_error_create, copy_item_to_temp, copy_rep_to_temp,

copy_node_to_temp, rep_write_get_baton, rep_write_contents_close,

write_container_rep, write_container_delta_rep, write_final_rev,

write_final_changed_path_info, commit_body, expect_buffer_nul): Replace

calls to svn_fs_fs__get_file_offset() within svn_io_file_get_offset().

  1. … 8 more files in changeset.
Revert r1719188: It seems APR doesn't report proper position for apr_file_t

with ungotten character while our patch parser relies on that.

* subversion/include/svn_io.h

(svn_io_file_ungetc): Revert r1719188.

* subversion/libsvn_subr/io.c

(svn_io_file_ungetc, svn_io_file_readline): Revert r1719188.

* subversion/tests/libsvn_subr/io-test.c

(test_file_readline, test_file_ungetc,

svn_test_descriptor_t): Revert r1719188.

  1. … 2 more files in changeset.
[Reverted in 1719196]

Use existing APR function in implementation of svn_io_file_readline() for

peeking char after we found '\r' instead of save position and seek back.

* subversion/include/svn_io.h

(svn_io_file_ungetc): New.

* subversion/libsvn_subr/io.c

(svn_io_file_ungetc): New. Wrapper around apr_file_ungetc().

(svn_io_file_readline): Use svn_io_file_ungetc() for peeking char after we

found '\r' instead of save position and seek back.

* subversion/tests/libsvn_subr/io-test.c

(test_file_readline): New test for svn_io_file_readline().

(test_file_ungetc): New test for svn_io_file_ungetc().

(svn_test_descriptor_t): Add test_file_readline and test_file_ungetc.

  1. … 2 more files in changeset.
When getting the default file permissions for a file created outside

the system directory create the necessary temporary files in the

given directory. This removes the disk IO to TMPDIR during the first

commit made by mod_dav_svn and svnserve processes, all disk IO now

happens in the repositories.

* subversion/libsvn_subr/io.c

(get_default_file_perms): Create temporary files in given directory.

(merge_default_file_perms): Add directory parameter.

(svn_io_open_unique_file3): Pass directory.

Implement svn_io_write_atomic2() with FLUSH_TO_DISK flag to control whether

wait or not until file is actually written to disk. The old

svn_io_write_atomic() was flushing data to disk unconditionally.

* subversion/include/svn_io.h

(svn_io_write_atomic2): New function declaration.

(svn_io_write_atomic): Deprecate.

* subversion/libsvn_subr/io.c

(svn_io_write_atomic2): Revv from svn_io_write_atomic() Add FLUSH_TO_DISK

parameter and perform flush to disk only if FLUSH_TO_DISK is non-zero.

* subversion/libsvn_subr/deprecated.c

(svn_io_write_atomic): Call svn_io_write_atomic2() with FLUSH_TO_DISK=TRUE.

* subversion/libsvn_fs_fs/fs_fs.c

* subversion/libsvn_fs_fs/transaction.c

* subversion/libsvn_fs_fs/util.c

* subversion/libsvn_fs_x/fs_x.c

* subversion/libsvn_fs_x/revprops.c

* subversion/libsvn_fs_x/util.c

* subversion/libsvn_wc/workqueue.c

* subversion/mod_dav_svn/activity.c

* subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c

* subversion/tests/libsvn_fs_x/fs-x-pack-test.c

* subversion/tests/libsvn_repos/repos-test.c

(*): Use svn_io_write_atomic2() with FLUSH_TO_DISK=TRUE instead of

svn_io_write_atomic().

  1. … 13 more files in changeset.