New patch follows at end of msg...
On Mon, Mar 17, 2003 at 10:24:50PM -0600, Karl Fogel wrote:
>
> I know it's a pain, but when you create a new function, it should
> explain all of its parameters. For example, can ADM_ACCESS be null or
> not? What precisely gets allocated in POOL? (And in what precise
> form is the data in PATTERNS stored?)
How 'precise' do you really have to be when, for example, this
function doesn't use POOL directly but just passes it to one or more
callees? (Another function in that file simply says 'Allocate everything
in POOL'... i've done the same in the patch below.)
> > - /* Copy default ignores into the local PATTERNS array. */
> > - patterns = apr_array_make (subpool, 1, sizeof(const char *));
> > - for (i = 0; i < ignores->nelts; i++)
> > - {
> > - const char *ignore = APR_ARRAY_IDX (ignores, i, const char *);
> > - (*((const char **) apr_array_push (patterns))) = ignore;
> > - }
> > -
> > - /* Then add any svn:ignore globs to the PATTERNS array. */
> > - SVN_ERR (add_ignore_patterns (adm_access, patterns, subpool));
> > + patterns = apr_array_make (pool, 1, sizeof(const char *));
> > + SVN_ERR (collect_ignore_patterns (patterns, ignores,
> > + adm_access, subpool));
>
> Hmmm. I'm not sure there's anything wrong here, but a minor red flag
> went up when I saw that `patterns' is allocated in `pool', but
> collect_ignore_patterns() takes `subpool', not `pool'. It seems like
> the lifetime effects in the new code are different from the old code,
> since the old code (visible above) just pushes stuff straight from
> `ignores' to `patterns', without involving a subpool.
AFAICT the old code allocates both PATTERNS and the ignore-patterns
added by add_ignore_patterns in SUBPOOL.
my slip-up was using POOL for the allocation of PATTERNS. i don't think
it would cause any harm, but SUBPOOL is better (if i'm understanding
pools correctly).
these issues, as well as your other comments, are addressed in the
following patch.
-brian
Log:
Resolves Issue #1076:
- print 'I' (not '?') for ignored files when named explicitly on the
command line, or when --no-ignores is used.
- print nothing for nonexistent files (unversioned and do not exist on disk)
* subversion/include/svn_wc.h
(enum svn_wc_status_kind): Add status kind svn_wc_status_ignored.
* subversion/libsvn_wc/status.c
(assemble_status): Set text_status to svn_wc_status_ignored
for ignored files.
(collect_ignore_patterns): New function, extracted from
add_unversioned_items.
(add_unversioned_item): New function, extracted from
add_unversioned_items.
(add_unversioned_items): Extract helper functions mentioned above.
(svn_wc_statuses): If there is no entry for PATH, handle it
correctly as an unversioned item.
* subversion/clients/cmdline/status.c
(generate_status_code): Return 'I' for svn_wc_status_ignored.
(svn_cl__print_status_list): Skip printing items whose status is
svn_wc_status_none.
* subversion/clients/cmdline/main.c
(svn_cl__cmd_table): Add 'I' to output of 'svn help status'.
* subversion/tests/clients/cmdline/stat_tests.py
(status_for_unignored_file)
[renamed from status_blank_for_unignored_file]: Check that text
status is 'I' for a file marked ignored, if specified explicitly
or --no-ignores is given.
(status_for_nonexistent_file): Check that no output is given for a
file that is not versioned and does not exist.
* doc/book/book/ch06.xml
Give example of using 'svn status --no-ignore'
* doc/book/book/ch08.xml
Add explanation of 'I' to 'svn status' reference
Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h (revision 5370)
+++ subversion/include/svn_wc.h (working copy)
@@ -841,6 +841,9 @@
/** local mods received conflicting repos mods */
svn_wc_status_conflicted,
+ /** a resource marked as ignored */
+ svn_wc_status_ignored,
+
/** an unversioned resource is in the way of the versioned resource */
svn_wc_status_obstructed
};
Index: subversion/libsvn_wc/status.c
===================================================================
--- subversion/libsvn_wc/status.c (revision 5370)
+++ subversion/libsvn_wc/status.c (working copy)
@@ -151,12 +151,12 @@
/* If this path has no entry, but IS present on disk, it's
unversioned. If this file is being explicitly ignored (due
to matching an ignore-pattern), the text_status is set to
- svn_wc_status_none. Otherwise the text_status is set to
+ svn_wc_status_ignored. Otherwise the text_status is set to
svn_wc_status_unversioned. */
if (path_kind != svn_node_none)
{
if (is_ignored)
- stat->text_status = svn_wc_status_none;
+ stat->text_status = svn_wc_status_ignored;
else
stat->text_status = svn_wc_status_unversioned;
}
@@ -363,6 +363,114 @@
}
+/* Store in PATTERNS a list of all svn:ignore properties from
+ the working copy directory, including the default ignores
+ passed in as IGNORES.
+
+ Upon return, *PATTERNS will contain zero or more (const char *)
+ patterns from the value of the SVN_PROP_IGNORE property set on
+ the working directory path.
+
+ IGNORES is a list of patterns to include; typically this will
+ be the default ignores as, for example, specified in a config file.
+
+ ADM_ACCESS is an access baton for the working copy path.
+
+ Allocate everything in POOL.
+
+ None of the arguments may be NULL.
+*/
+static svn_error_t *
+collect_ignore_patterns (apr_array_header_t *patterns,
+ apr_array_header_t *ignores,
+ svn_wc_adm_access_t *adm_access,
+ apr_pool_t *pool)
+{
+ int i;
+
+ /* Copy default ignores into the local PATTERNS array. */
+ for (i = 0; i < ignores->nelts; i++)
+ {
+ const char *ignore = APR_ARRAY_IDX (ignores, i, const char *);
+ (*((const char **) apr_array_push (patterns))) = ignore;
+ }
+
+ /* Then add any svn:ignore globs to the PATTERNS array. */
+ SVN_ERR (add_ignore_patterns (adm_access, patterns, pool));
+
+ return SVN_NO_ERROR;
+}
+
+
+/* Add a status_structure for BASENAME to the STATUSHASH, assuming
+ that the file is unversioned. This function should never
+ be called on a versioned entry.
+
+ BASENAME is the basename of the unversioned file whose status is being
+ requested.
+
+ PATH_KIND is the node kind of PATH as determined by the caller.
+
+ STATUSHASH is a mapping from path to status structure. On entry, it
+ may or may not contain status structures for other paths. Upon return
+ it may contain a status structure for BASENAME.
+
+ ADM_ACCESS is an access baton for the working copy path.
+
+ PATTERNS points to a list of filename patterns which are marked
+ as ignored.
+
+ None of the above arguments may be NULL.
+
+ If NO_IGNORE is non-zero, the item will be added regardless of whether
+ it is ignored; otherwise we will only add the item if it does not
+ match any of the patterns in IGNORES.
+
+ If a status structure for the item is added, NOTIFY_FUNC will called
+ with the path of the item and the NOTIFY_BATON. NOTIFY_FUNC may be
+ NULL if no such notification is required.
+
+ Allocate everything in POOL.
+*/
+static svn_error_t *
+add_unversioned_item (const char *basename,
+ svn_node_kind_t path_kind,
+ apr_hash_t *statushash,
+ svn_wc_adm_access_t *adm_access,
+ apr_array_header_t *patterns,
+ svn_boolean_t no_ignore,
+ svn_wc_notify_func_t notify_func,
+ void *notify_baton,
+ apr_pool_t *pool)
+{
+ int ignore_me;
+ const char *printable_path;
+
+ ignore_me = svn_cstring_match_glob_list (basename, patterns);
+
+ /* If we aren't ignoring it, add a status structure for this dirent. */
+ if (no_ignore || ! ignore_me)
+ {
+ printable_path = svn_path_join (svn_wc_adm_access_path (adm_access),
+ basename, pool);
+
+ /* Add this item to the status hash. */
+ SVN_ERR (add_status_structure (statushash,
+ printable_path,
+ adm_access,
+ NULL, /* no entry */
+ NULL,
+ path_kind,
+ FALSE,
+ ignore_me, /* is_ignored */
+ notify_func,
+ notify_baton,
+ pool));
+ }
+
+ return SVN_NO_ERROR;
+}
+
/* Add all items that are NOT in ENTRIES (which is a list of PATH's
versioned things) to the STATUSHASH as unversioned items,
@@ -386,7 +494,7 @@
a pattern in IGNORES, but which are being represented in
status structures anyway because the caller has explicitly
requested _all_ items. (These ultimately get their
- text_status set to svn_wc_status_none.)
+ text_status set to svn_wc_status_ignored.)
*/
static svn_error_t *
add_unversioned_items (svn_wc_adm_access_t *adm_access,
@@ -411,18 +519,9 @@
to add any svn:ignore properties from the parent directory. */
if (ignores)
{
- int i;
-
- /* Copy default ignores into the local PATTERNS array. */
patterns = apr_array_make (subpool, 1, sizeof(const char *));
- for (i = 0; i < ignores->nelts; i++)
- {
- const char *ignore = APR_ARRAY_IDX (ignores, i, const char *);
- (*((const char **) apr_array_push (patterns))) = ignore;
- }
-
- /* Then add any svn:ignore globs to the PATTERNS array. */
- SVN_ERR (add_ignore_patterns (adm_access, patterns, subpool));
+ SVN_ERR (collect_ignore_patterns (patterns, ignores,
+ adm_access, subpool));
}
else
patterns = NULL;
@@ -434,8 +533,6 @@
apr_ssize_t klen;
void *val;
const char *keystring;
- int ignore_me;
- const char *printable_path;
svn_node_kind_t *path_kind;
apr_hash_this (hi, &key, &klen, &val);
@@ -450,28 +547,9 @@
if (! strcmp (keystring, SVN_WC_ADM_DIR_NAME))
continue;
- ignore_me = svn_cstring_match_glob_list (keystring, patterns);
-
- /* If we aren't ignoring it, add a status structure for this
- dirent. */
- if (no_ignore || ! ignore_me)
- {
- printable_path = svn_path_join (svn_wc_adm_access_path (adm_access),
- keystring, pool);
-
- /* Add this item to the status hash. */
- SVN_ERR (add_status_structure (statushash,
- printable_path,
- adm_access,
- NULL, /* no entry */
- NULL,
- *path_kind,
- FALSE,
- ignore_me, /* is_ignored */
- notify_func,
- notify_baton,
- pool));
- }
+ SVN_ERR (add_unversioned_item (keystring, *path_kind, statushash,
+ adm_access, patterns, no_ignore,
+ notify_func, notify_baton, pool));
}
svn_pool_destroy (subpool);
@@ -667,7 +745,9 @@
/* Is PATH a directory or file? */
SVN_ERR (svn_io_check_path (path, &kind, pool));
- /* Read the appropriate entries file */
+ /* Read the default ignores from the config hash. */
+ apr_array_header_t *ignores;
+ SVN_ERR (svn_wc_get_default_ignores (&ignores, config, pool));
/* If path points to just one file, or at least to just one
non-directory, store just one status structure in the
@@ -678,18 +758,35 @@
/* Get the entry for this file. Place it into the specified pool since
we're going to return it in statushash. */
SVN_ERR (svn_wc_entry (&entry, path, adm_access, FALSE, pool));
- SVN_ERR (svn_wc_entry (&parent_entry,
- svn_path_dirname (path,pool),
- adm_access, FALSE, pool));
/* Convert the entry into a status structure, store in the hash.
### Notice that because we're getting one specific file,
we're ignoring the GET_ALL flag and unconditionally fetching
the status structure. */
- SVN_ERR (add_status_structure (statushash, path, adm_access, entry,
- parent_entry, kind, TRUE, FALSE,
- notify_func, notify_baton, pool));
+ if (!entry)
+ {
+ char *basename;
+ apr_array_header_t *patterns;
+
+ patterns = apr_array_make (pool, 1, sizeof(const char *));
+ SVN_ERR (collect_ignore_patterns (patterns, ignores,
+ adm_access, pool));
+
+ basename = svn_path_basename (path, pool);
+ SVN_ERR (add_unversioned_item (basename, kind, statushash,
+ adm_access, patterns, TRUE,
+ notify_func, notify_baton, pool));
+ }
+ else
+ {
+ SVN_ERR (svn_wc_entry (&parent_entry,
+ svn_path_dirname (path,pool),
+ adm_access, FALSE, pool));
+ SVN_ERR (add_status_structure (statushash, path, adm_access, entry,
+ parent_entry, kind, TRUE, FALSE,
+ notify_func, notify_baton, pool));
+ }
}
/* Fill the hash with a status structure for *each* entry in PATH */
@@ -698,7 +795,6 @@
int wc_format_version;
svn_boolean_t is_root;
const svn_wc_entry_t *parent_entry;
- apr_array_header_t *ignores;
SVN_ERR (svn_wc_check_wc (path, &wc_format_version, pool));
@@ -722,9 +818,6 @@
else
parent_entry = NULL;
- /* Read the default ignores from the config files. */
- SVN_ERR (svn_wc_get_default_ignores (&ignores, config, pool));
-
SVN_ERR (get_dir_status(statushash, parent_entry, adm_access,
ignores, descend, get_all, no_ignore,
notify_func, notify_baton,
Index: subversion/clients/cmdline/status.c
===================================================================
--- subversion/clients/cmdline/status.c (revision 5370)
+++ subversion/clients/cmdline/status.c (working copy)
@@ -45,6 +45,7 @@
case svn_wc_status_merged: return 'G';
case svn_wc_status_conflicted: return 'C';
case svn_wc_status_obstructed: return '~';
+ case svn_wc_status_ignored: return 'I';
case svn_wc_status_unversioned: return '?';
default: return '?';
}
@@ -164,7 +165,9 @@
item = &APR_ARRAY_IDX(statusarray, i, const svn_item_t);
status = item->value;
- if (! status || (skip_unrecognized && ! status->entry))
+ if (! status
+ || (skip_unrecognized && ! status->entry)
+ || (status->text_status == svn_wc_status_none))
continue;
err = svn_utf_cstring_from_utf8 (&path, item->key, pool);
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c (revision 5370)
+++ subversion/clients/cmdline/main.c (working copy)
@@ -414,6 +414,7 @@
" 'D' Deleted\n"
" 'M' Modified\n"
" 'C' Conflicted\n"
+ " 'I' Ignored\n"
" '?' item is not under revision control\n"
" '!' item is missing and was removed via a non-svn command\n"
" '~' versioned item obstructed by some item of a different kind\n"
Index: subversion/tests/clients/cmdline/stat_tests.py
===================================================================
--- subversion/tests/clients/cmdline/stat_tests.py (revision 5370)
+++ subversion/tests/clients/cmdline/stat_tests.py (working copy)
@@ -314,33 +314,75 @@
return 0
-def status_blank_for_unignored_file(sbox):
- "status blank for unignored file"
+def status_for_unignored_file(sbox):
+ "status for unignored file"
- if sbox.build():
- return 1
+ sbox.build()
wc_dir = sbox.wc_dir
was_cwd = os.getcwd()
os.chdir(wc_dir)
- svntest.main.file_append('newfile', 'this is a new file')
- svntest.main.run_svn(None, 'propset', 'svn:ignore', 'newfile', '.')
- stat_output, err_output = svntest.main.run_svn(None, 'status', '--no-ignore',
- '.')
- if err_output:
- return 1
- status = 1
- for line in stat_output:
- if re.match(" +newfile", line):
- status = 0
+ try:
+ svntest.main.file_append('newfile', 'this is a new file')
+ svntest.main.run_svn(None, 'propset', 'svn:ignore', 'newfile', '.')
+
+ # status on the directory with --no-ignore
+ stat_output, err_output = svntest.main.run_svn(None, 'status',
+ '--no-ignore', '.')
+ if err_output:
+ raise svntest.Failure
+ status = 1
+ for line in stat_output:
+ if re.match("I +newfile", line):
+ status = 0
+
+ if (status == 1):
+ raise svntest.Failure
+
+ # status specifying the file explicitly on the command line
+ stat_output, err_output = svntest.main.run_svn(None, 'status', 'newfile')
+
+ if err_output:
+ raise svntest.Failure
+ status = 1
+ for line in stat_output:
+ if re.match("I +newfile", line):
+ status = 0
+
+ if (status == 1):
+ raise svntest.Failure
- os.chdir(was_cwd)
+ finally:
+ os.chdir(was_cwd)
- return status
+def status_for_nonexistent_file(sbox):
+ "status for a file neither on disk nor under version control"
+ sbox.build()
+
+ wc_dir = sbox.wc_dir
+ was_cwd = os.getcwd()
+
+ os.chdir(wc_dir)
+
+ try:
+ stat_output, err_output = svntest.main.run_svn(None, 'status',
+ 'nonexistent-file')
+ if err_output:
+ raise svntest.Failure
+
+ # there should *not* be a status line printed for the nonexistent file
+ for line in stat_output:
+ if re.match(" +nonexistent-file", line):
+ raise svntest.Failure
+
+ finally:
+ os.chdir(was_cwd)
+
+
def status_file_needs_update(sbox):
"status -u should show that outdated file needs update"
@@ -486,7 +528,8 @@
status_type_change,
Skip(status_type_change_to_symlink, (os.name != 'posix')),
status_with_new_files_pending,
- status_blank_for_unignored_file,
+ status_for_unignored_file,
+ status_for_nonexistent_file,
status_file_needs_update,
XFail(status_uninvited_parent_directory),
]
Index: doc/book/book/ch06.xml
===================================================================
--- doc/book/book/ch06.xml (revision 5370)
+++ doc/book/book/ch06.xml (working copy)
@@ -1145,6 +1145,20 @@
items— such as that source code file that you probably
forgot to add to version control.</para>
+ <para>If you want to see the ignored files, you can pass the
+ <command>--no-ignore</command> option to subversion:</para>
+
+<screen>
+$ svn status --no-ignore
+ M calc/button.c
+I calc/calculator
+? calc/data.c
+I calc/debug_log
+I calc/debug_log.1
+I calc/debug_log.2.gz
+I calc/debug_log.3.gz
+</screen>
+
</sect3>
<!-- cmpilato was here -->
Index: doc/book/book/ch08.xml
===================================================================
--- doc/book/book/ch08.xml (revision 5370)
+++ doc/book/book/ch08.xml (working copy)
@@ -2716,6 +2716,14 @@
</varlistentry>
<varlistentry>
+ <term>'I'</term>
+ <listitem>
+ <para>Item is being ignored (e.g. with the
+ svn:ignore) property</para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term>'?'</term>
<listitem>
<para>Item is not under version control.</para>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Mar 18 20:31:31 2003