Re: [PATCH] issue #4375: provide --password-fd option
Hey there!
Sorry for the long delay! I've finally amended the patch to make the
recommended fixes.
Thanks
Index: subversion/include/svn_io.h
===
--- subversion/include/svn_io.h (revisión: 1818629)
+++ subversion/include/svn_io.h (copia de trabajo)
@@ -2633,6 +2633,15 @@
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
+/** Reads a string from stdin until a newline or EOF is found
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_io_stdin_readline(const char **result,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
/** @} */
#ifdef __cplusplus
Index: subversion/libsvn_subr/io.c
===
--- subversion/libsvn_subr/io.c (revisión: 1818629)
+++ subversion/libsvn_subr/io.c (copia de trabajo)
@@ -5440,3 +5440,20 @@
return SVN_NO_ERROR;
}
+
+svn_error_t *
+svn_io_stdin_readline(const char **result,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ svn_stringbuf_t *buf = NULL;
+ svn_stream_t *stdin = NULL;
+ svn_boolean_t oob = FALSE;
+
+ SVN_ERR(svn_stream_for_stdin2(, TRUE, scratch_pool));
+ SVN_ERR(svn_stream_readline(stdin, , APR_EOL_STR, , result_pool));
+
+ *result = buf->data;
+
+ return SVN_NO_ERROR;
+}
Index: subversion/svn/cl.h
===
--- subversion/svn/cl.h (revisión: 1818629)
+++ subversion/svn/cl.h (copia de trabajo)
@@ -178,6 +178,7 @@
svn_boolean_t help;/* print usage message */
const char *auth_username; /* auth username */
const char *auth_password; /* auth password */
+ svn_boolean_t auth_password_from_stdin; /* read password from stdin */
const char *extensions;/* subprocess extension args */
apr_array_header_t *targets; /* target list from file */
svn_boolean_t xml; /* output in xml, e.g., "svn log --xml" */
Index: subversion/svn/svn.c
===
--- subversion/svn/svn.c (revisión: 1818629)
+++ subversion/svn/svn.c (copia de trabajo)
@@ -68,6 +68,7 @@
use the short option letter as identifier. */
typedef enum svn_cl__longopt_t {
opt_auth_password = SVN_OPT_FIRST_LONGOPT_ID,
+ opt_auth_password_from_stdin,
opt_auth_username,
opt_autoprops,
opt_changelist,
@@ -203,6 +204,9 @@
N_("specify a password ARG (caution: on many operating\n"
" "
"systems, other users will be able to see this)")},
+ {"password-from-stdin",
+opt_auth_password_from_stdin, 0,
+N_("read password from stdin")},
{"extensions",'x', 1,
N_("Specify differencing options for external diff or\n"
" "
@@ -504,7 +508,8 @@
command to take these arguments allows scripts to just pass them
willy-nilly to every invocation of 'svn') . */
const int svn_cl__global_options[] =
-{ opt_auth_username, opt_auth_password, opt_no_auth_cache, opt_non_interactive,
+{ opt_auth_username, opt_auth_password, opt_auth_password_from_stdin,
+ opt_no_auth_cache, opt_non_interactive,
opt_force_interactive, opt_trust_server_cert,
opt_trust_server_cert_failures,
opt_config_dir, opt_config_options, 0
@@ -2036,6 +2041,7 @@
apr_hash_t *changelists;
apr_hash_t *cfg_hash;
svn_membuf_t buf;
+ svn_boolean_t read_pass_from_stdin = FALSE;
received_opts = apr_array_make(pool, SVN_OPT_MAX_OPTIONS, sizeof(int));
@@ -2331,6 +2337,9 @@
SVN_ERR(svn_utf_cstring_to_utf8(_state.auth_password,
opt_arg, pool));
break;
+ case opt_auth_password_from_stdin:
+read_pass_from_stdin = TRUE;
+break;
case opt_encoding:
opt_state.encoding = apr_pstrdup(pool, opt_arg);
break;
@@ -2827,6 +2836,14 @@
"--non-interactive"));
}
+ /* --password-from-stdin can only be used with --non-interactive */
+ if (read_pass_from_stdin && !opt_state.non_interactive)
+{
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("--password-from-stdin requires "
+"--non-interactive"));
+}
+
/* Disallow simultaneous use of both --diff-cmd and
--internal-diff. */
if (opt_state.diff.diff_cmd && opt_state.diff.internal_diff)
@@ -3117,6 +3134,12 @@
conflict_stats, pool));
}
+ /* Get password from stdin if necessary */
+ if (read_pass_from_stdin)
+{
+ SVN_ERR(svn_io_stdin_readline(_state.auth_password, pool, pool));
+}
+
/* Set up our cancellation support. */
svn_cl__check_cancel
Re: [PATCH] release.py: write-changelog function (POC)
After some further thought and tweaking, I committed a write-changelog function in r1818988. I tried to make it flexible enough so we can experiment a bit with it. - The "changes labels" can be used as prefix or suffix. - They can be [U:client], [D:api], ... or [U], [D], or simply [] - There's an option --include-unlabeled-summaries to include summary lines without a label, unless [skip], [ignore], [c:skip] or [c:ignore] is found somewhere in the commit message. Summary lines containing 'STATUS', 'CHANGES', 'Post-release housekeeping' or 'Follow-up' are ignored. I guess further tweaks will come up if / when we start to use this. Also, feel free to improve the code ... I'm far from a python expert (depending heavily on copy-paste and StackOverflow). We might need to do something special with the words 'Revert' and 'Merge', but I'm not yet sure what. -- Johan
RE: svn_dirent_t.size API inconsistency
> -Original Message-
> From: Stefan Fuhrmann [mailto:stef...@apache.org]
> Sent: donderdag 21 december 2017 16:01
> To: dev@subversion.apache.org
> Subject: Re: svn_dirent_t.size API inconsistency
>
> On 21.12.2017 15:14, Stefan Fuhrmann wrote:
> > I think I found an API documentation bug.
> > svn_types.h specifies for svn_dirent_t.size:
> >
> > /** length of file text, or 0 for directories */
> > svn_filesize_t size;
> >
> > However, (almost?) all implementations set it to
> > SVN_INVALID_FILESIZE for directories. This is also
> > what we do for svn_io_dirent_t.size.
> >
> > I discovered this as r1818578 broke our Perl tests.
> > The same test makes it clear that no API user could
> > have successfully used the API as documented.
> >
> > Therefore, I suggest to change the API doc to match
> > the implementation, check the code for consistency
> > and add an entry to our errata list.
+1 on the change.
Given that we already have all existing callers do the right thing (so no
behavior change), I'm not sure if adding something to our errata list is
necessary... But I don't see a problem with adding it anyway.
Bert
> >
> > -- Stefan^2.
> >
> Below the necessary patch w/o erratum.
>
> -- Stefan^2.
>
> [[[
> Fix inconsistent handling of svn_dirent_t.size for directories.
>
> Most code uses SVN_INVALID_FILESIZE for them, so change the docs to
> match
> that fact and update all users to consistently follow the new docstring.
>
> * subversion/include/svn_types.h
>(svn_dirent_t): Change documentation for SIZE.
>
> * subversion/libsvn_ra_local/ra_plugin.c
>(svn_ra_local__get_dir): Return the new default for directories.
>
> * subversion/libsvn_ra_serf/list.c
>(item_closed): Be sure to default to SVN_INVALID_FILESIZE.
>
> * subversion/libsvn_repos/list.c
>(fill_dirent): Set SIZE consistently with other RA layers.
>
> --This line, and those below, will be ignored--
>
> Index: subversion/include/svn_types.h
> ===
>
> --- subversion/include/svn_types.h(revision 1818804)
> +++ subversion/include/svn_types.h(working copy)
> @@ -652,7 +652,7 @@ typedef struct svn_dirent_t
> /** node kind */
> svn_node_kind_t kind;
>
> - /** length of file text, or 0 for directories */
> + /** length of file text, otherwise SVN_INVALID_FILESIZE */
> svn_filesize_t size;
>
> /** does the node have props? */
> Index: subversion/libsvn_ra_local/ra_plugin.c
> ===
>
> --- subversion/libsvn_ra_local/ra_plugin.c(revision 1818804)
> +++ subversion/libsvn_ra_local/ra_plugin.c(working copy)
> @@ -1387,7 +1387,7 @@ svn_ra_local__get_dir(svn_ra_session_t *
> {
> /* size */
> if (fs_entry->kind == svn_node_dir)
> -entry->size = 0;
> +entry->size = SVN_INVALID_FILESIZE;
> else
> SVN_ERR(svn_fs_file_length(&(entry->size), root,
> fullpath, iterpool));
> Index: subversion/libsvn_ra_serf/list.c
> ===
>
> --- subversion/libsvn_ra_serf/list.c(revision 1818932)
> +++ subversion/libsvn_ra_serf/list.c(working copy)
> @@ -139,6 +139,8 @@ item_closed(svn_ra_serf__xml_estate_t *x
>
> if (size)
> SVN_ERR(svn_cstring_atoi64(, size));
> + else
> +dirent.size = SVN_INVALID_FILESIZE;
>
> if (crev)
> SVN_ERR(svn_revnum_parse(_rev, crev, NULL));
> Index: subversion/libsvn_repos/list.c
> ===
>
> --- subversion/libsvn_repos/list.c(revision 1818804)
> +++ subversion/libsvn_repos/list.c(working copy)
> @@ -51,7 +51,7 @@ fill_dirent(svn_dirent_t *dirent,
> if (dirent->kind == svn_node_file)
> SVN_ERR(svn_fs_file_length(&(dirent->size), root, path,
> scratch_pool));
> else
> -dirent->size = 0;
> +dirent->size = SVN_INVALID_FILESIZE;
>
> SVN_ERR(svn_fs_node_has_props(>has_props, root, path,
> scratch_pool));
> ]]]
>
Re: svn_dirent_t.size API inconsistency
On 21.12.2017 15:14, Stefan Fuhrmann wrote:
I think I found an API documentation bug.
svn_types.h specifies for svn_dirent_t.size:
/** length of file text, or 0 for directories */
svn_filesize_t size;
However, (almost?) all implementations set it to
SVN_INVALID_FILESIZE for directories. This is also
what we do for svn_io_dirent_t.size.
I discovered this as r1818578 broke our Perl tests.
The same test makes it clear that no API user could
have successfully used the API as documented.
Therefore, I suggest to change the API doc to match
the implementation, check the code for consistency
and add an entry to our errata list.
-- Stefan^2.
Below the necessary patch w/o erratum.
-- Stefan^2.
[[[
Fix inconsistent handling of svn_dirent_t.size for directories.
Most code uses SVN_INVALID_FILESIZE for them, so change the docs to match
that fact and update all users to consistently follow the new docstring.
* subversion/include/svn_types.h
(svn_dirent_t): Change documentation for SIZE.
* subversion/libsvn_ra_local/ra_plugin.c
(svn_ra_local__get_dir): Return the new default for directories.
* subversion/libsvn_ra_serf/list.c
(item_closed): Be sure to default to SVN_INVALID_FILESIZE.
* subversion/libsvn_repos/list.c
(fill_dirent): Set SIZE consistently with other RA layers.
--This line, and those below, will be ignored--
Index: subversion/include/svn_types.h
===
--- subversion/include/svn_types.h (revision 1818804)
+++ subversion/include/svn_types.h (working copy)
@@ -652,7 +652,7 @@ typedef struct svn_dirent_t
/** node kind */
svn_node_kind_t kind;
- /** length of file text, or 0 for directories */
+ /** length of file text, otherwise SVN_INVALID_FILESIZE */
svn_filesize_t size;
/** does the node have props? */
Index: subversion/libsvn_ra_local/ra_plugin.c
===
--- subversion/libsvn_ra_local/ra_plugin.c (revision 1818804)
+++ subversion/libsvn_ra_local/ra_plugin.c (working copy)
@@ -1387,7 +1387,7 @@ svn_ra_local__get_dir(svn_ra_session_t *
{
/* size */
if (fs_entry->kind == svn_node_dir)
- entry->size = 0;
+ entry->size = SVN_INVALID_FILESIZE;
else
SVN_ERR(svn_fs_file_length(&(entry->size), root,
fullpath, iterpool));
Index: subversion/libsvn_ra_serf/list.c
===
--- subversion/libsvn_ra_serf/list.c (revision 1818932)
+++ subversion/libsvn_ra_serf/list.c (working copy)
@@ -139,6 +139,8 @@ item_closed(svn_ra_serf__xml_estate_t *x
if (size)
SVN_ERR(svn_cstring_atoi64(, size));
+ else
+ dirent.size = SVN_INVALID_FILESIZE;
if (crev)
SVN_ERR(svn_revnum_parse(_rev, crev, NULL));
Index: subversion/libsvn_repos/list.c
===
--- subversion/libsvn_repos/list.c (revision 1818804)
+++ subversion/libsvn_repos/list.c (working copy)
@@ -51,7 +51,7 @@ fill_dirent(svn_dirent_t *dirent,
if (dirent->kind == svn_node_file)
SVN_ERR(svn_fs_file_length(&(dirent->size), root, path,
scratch_pool));
else
- dirent->size = 0;
+ dirent->size = SVN_INVALID_FILESIZE;
SVN_ERR(svn_fs_node_has_props(>has_props, root, path,
scratch_pool));
]]]
svn_dirent_t.size API inconsistency
I think I found an API documentation bug. svn_types.h specifies for svn_dirent_t.size: /** length of file text, or 0 for directories */ svn_filesize_t size; However, (almost?) all implementations set it to SVN_INVALID_FILESIZE for directories. This is also what we do for svn_io_dirent_t.size. I discovered this as r1818578 broke our Perl tests. The same test makes it clear that no API user could have successfully used the API as documented. Therefore, I suggest to change the API doc to match the implementation, check the code for consistency and add an entry to our errata list. -- Stefan^2.
Re: list --search matching and Windows *-expansion
On 21.12.2017 14:50, Stefan Fuhrmann wrote: > On 19.12.2017 11:06, Stefan Sperling wrote: >> On Mon, Dec 18, 2017 at 08:59:08PM +, Julian Foad wrote: >>> Stefan Sperling wrote: On Mon, Dec 18, 2017 at 06:01:52PM +, Daniel Shahaf wrote: > Branko Čibej wrote on Mon, 18 Dec 2017 16:24 +0100: >> Or use a different set of match chars on Windows instead of * and >> ?, as >> a hacky but usable workaround. [...] >>> [...] > a magic prefix, so «--search "nostar:foo#bar%"» [...] >>> [...] define some mapping between our fnmatch special chars [...] Windows users would [...] use the set of chars which works for them. >>> Reality check, please! >>> >>> This is meant to be a simple feature to help people find files, is >>> it not? >>> >>> Magic prefixes and non-standard wildcard chars are a non-starter for >>> usability. >>> >>> It would be better to disable the whole '--search' option, or the >>> wildcard >>> support in it, on the Windows CLI. >> I think we should keep coming up with more ideas until somebody gets >> enough >> of it all and sends a patch to remove our dependency on setargv.obj :) >> > Well, that escalated quickly ;) > > While I liked Brane's idea for being easy to implement, > I think Julian's point concerning usability is stronger. > Therefore, I will go with my initial proposal to make > 'svn ls --search' in the Windows CLI work the same as > 'svn log --search'. > > Given that we have fnmatch available, replacing setargv.obj > in 1.11 should not be too much of an effort. I don't have > access to a Windows environment ATM, though. It's not as simple as all that. We should not just replace setargv.obj but also implement some kind of quoting (preferably compatible with the Windows command line in general). Otherwise the wildcard expansion logic will bleed into the argument interpretation, and we definitely do not want to fill the command-line code with a mountain of #ifdefs. -- Brane
