Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow --print-stats --null-output #649

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Allow --print-stats --null-output #649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5afad41
Pass ${print_nulls} to chunks_stats_print*
gperciva Feb 24, 2025
334c114
chunks_stats_print*: print csv fields separately
gperciva Feb 24, 2025
4208028
Add --null-output for --print-stats
gperciva Feb 24, 2025
5fb57f8
man, shell description: --print-stats --null-output
gperciva Feb 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
separate each filename with a null character. If there are multiple fields
on a line, null characters are used instead of spaces; see the man page for
details.
- tarsnap now accepts --null-output with --print-stats, which causes it to
separate fields and lines with null character(s); see the man page for
details.
- tarsnap now accepts --null-input as a synonym for --null. For compatibility
reasons, --null is still supported, and will not be deprecated.
- tarsnap now accepts --hashes, which causes --list-archives to print hashes
Expand Down
3 changes: 2 additions & 1 deletion lib/util/print_separator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
* print ${num_nulls} '\0'.
*/
int
print_separator(FILE * stream, char * separator, int print_nulls, int num_nulls)
print_separator(FILE * stream, const char * separator, int print_nulls,
int num_nulls)
{
int i;

Expand Down
2 changes: 1 addition & 1 deletion lib/util/print_separator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
* Print ${separator} to ${stream} if ${print_nulls} is zero; otherwise,
* print ${num_nulls} '\0'.
*/
int print_separator(FILE *, char *, int, int);
int print_separator(FILE *, const char *, int, int);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. In #643, I was so fixated on whether we'd allow #define print_sep that I missed that the API should take const.


#endif /* !PRINT_SEPARATOR_H_ */
1 change: 1 addition & 0 deletions misc/zsh_completion/_tarsnap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ _shtab_tarsnap__print_stats_options=(
"--cachedir[specify cache directory]:cache-dir:{_files -/}"
"-f[specify name of archive to operate on]:archive-name:(${archive_list}):"
"--keep-going[keep going after missing an archive]"
"--null-output[output split by NULs instead of newlines]"
"--archive-names[read a list of archive names from a file]:filename:{_files}"
"--configfile[add to the list of config files to be read]:filename:{_files}"
"--csv-file[write statistics in CSV format to a file]:filename:{_files}"
Expand Down
4 changes: 3 additions & 1 deletion tar/bsdtar.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1066,7 +1066,9 @@ main(int argc, char **argv)
if (bsdtar->option_null_input)
only_mode(bsdtar, "--null-input", "cxt");
if (bsdtar->option_null_output) {
if (bsdtar->mode != OPTION_LIST_ARCHIVES)
/* Allow in --list-archives, --print-stats, and xt modes. */
if ((bsdtar->mode != OPTION_LIST_ARCHIVES) &&
(bsdtar->mode != OPTION_PRINT_STATS))
only_mode(bsdtar, "--null-output", "xt");
}

Expand Down
12 changes: 6 additions & 6 deletions tar/chunks/chunks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,18 +204,18 @@ CHUNKS_S * chunks_stats_init(const char *);
size_t chunks_stats_getdirsz(CHUNKS_S *);

/**
* chunks_stats_printglobal(stream, C, csv):
* chunks_stats_printglobal(stream, C, csv, print_nulls):
* Print global statistics relating to a set of archives, optionally in ${csv}
* format.
* format. If ${print_nulls} is non-zero, use '\0' as separators.
*/
int chunks_stats_printglobal(FILE *, CHUNKS_S *, int);
int chunks_stats_printglobal(FILE *, CHUNKS_S *, int, int);

/**
* chunks_stats_printarchive(stream, C, name, csv):
* chunks_stats_printarchive(stream, C, name, csv, print_nulls):
* Print accumulated statistics for an archive with the given name, optionally
* in ${csv} format.
* in ${csv} format. If ${print_nulls} is non-zero, use '\0' as separators.
*/
int chunks_stats_printarchive(FILE *, CHUNKS_S *, const char *, int);
int chunks_stats_printarchive(FILE *, CHUNKS_S *, const char *, int, int);

/**
* chunks_stats_free(C):
Expand Down
10 changes: 5 additions & 5 deletions tar/chunks/chunks_delete.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,21 +163,21 @@ chunks_delete_printstats(FILE * stream, CHUNKS_D * C, const char * name,
name = "This archive";

/* Print header. */
if (chunks_stats_printheader(stream, csv))
if (chunks_stats_printheader(stream, csv, 0))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we wanted to allow tarsnap -d --null-output -f archivename, then we'd hook it in here.

A small part of me thinks "hey, why not allow --null-output everywhere!". But a larger part of me points out that it's not needed for the GUI, and I struggle to think of any scenario where -d --null-output would really be useful.

goto err0;

/* Print the statistics we have. */
if (chunks_stats_print(stream, &C->stats_total, "All archives",
&C->stats_extra, csv))
&C->stats_extra, csv, 0))
goto err0;
if (chunks_stats_print(stream, &C->stats_unique, " (unique data)",
&C->stats_extra, csv))
&C->stats_extra, csv, 0))
goto err0;
if (chunks_stats_print(stream, &C->stats_tape, name,
&C->stats_tapee, csv))
&C->stats_tapee, csv, 0))
goto err0;
if (chunks_stats_print(stream, &C->stats_freed, "Deleted data",
&C->stats_tapee, csv))
&C->stats_tapee, csv, 0))
goto err0;

/* Success! */
Expand Down
13 changes: 7 additions & 6 deletions tar/chunks/chunks_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,19 @@ void chunks_stats_add(struct chunkstats *, size_t, size_t, ssize_t);
void chunks_stats_addstats(struct chunkstats *, struct chunkstats *);

/**
* chunks_stats_printheader(stream, csv):
* chunks_stats_printheader(stream, csv, print_nulls):
* Print a header line for statistics to ${stream}, optionally in ${csv}
* format.
* format. If ${print_nulls} is non-zero, use '\0' for separators.
*/
int chunks_stats_printheader(FILE *, int);
int chunks_stats_printheader(FILE *, int, int);

/**
* chunks_stats_print(stream, stats, name, stats_extra, csv):
* chunks_stats_print(stream, stats, name, stats_extra, csv, print_nulls):
* Print a line with ${name} and combined statistics from ${stats} and
* ${stats_extra} to ${stream}, optionally in ${csv} format.
* ${stats_extra} to ${stream}, optionally in ${csv} format. If ${print_nulls}
* is non-zero, use '\0' as separators.
*/
int chunks_stats_print(FILE *, struct chunkstats *, const char *,
struct chunkstats *, int);
struct chunkstats *, int, int);

#endif /* !CHUNKS_INTERNAL_H_ */
22 changes: 11 additions & 11 deletions tar/chunks/chunks_stats.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,24 +277,24 @@ chunks_stats_getdirsz(CHUNKS_S * C)
}

/**
* chunks_stats_printglobal(stream, C, csv):
* chunks_stats_printglobal(stream, C, csv, print_nulls):
* Print global statistics relating to a set of archives, optionally in ${csv}
* format.
* format. If ${print_nulls} is non-zero, use '\0' as separators.
*/
int
chunks_stats_printglobal(FILE * stream, CHUNKS_S * C, int csv)
chunks_stats_printglobal(FILE * stream, CHUNKS_S * C, int csv, int print_nulls)
{

/* Print header. */
if (chunks_stats_printheader(stream, csv))
if (chunks_stats_printheader(stream, csv, print_nulls))
goto err0;

/* Print the global statistics. */
if (chunks_stats_print(stream, &C->stats_total, "All archives",
&C->stats_extra, csv))
&C->stats_extra, csv, print_nulls))
goto err0;
if (chunks_stats_print(stream, &C->stats_unique, " (unique data)",
&C->stats_extra, csv))
&C->stats_extra, csv, print_nulls))
goto err0;

/* Success! */
Expand Down Expand Up @@ -375,21 +375,21 @@ chunks_stats_extrastats(CHUNKS_S * C, size_t len)
}

/**
* chunks_stats_printarchive(stream, C, name, csv):
* chunks_stats_printarchive(stream, C, name, csv, print_nulls):
* Print accumulated statistics for an archive with the given name, optionally
* in ${csv} format.
* in ${csv} format. If ${print_nulls} is non-zero, use '\0' as separators.
*/
int
chunks_stats_printarchive(FILE * stream, CHUNKS_S * C, const char * name,
int csv)
int csv, int print_nulls)
{

/* Print statistics for this archive. */
if (chunks_stats_print(stream, &C->stats_tape, name,
&C->stats_tapee, csv))
&C->stats_tapee, csv, print_nulls))
goto err0;
if (chunks_stats_print(stream, &C->stats_tapeu, " (unique data)",
&C->stats_tapee, csv))
&C->stats_tapee, csv, print_nulls))
goto err0;

/* Success! */
Expand Down
76 changes: 55 additions & 21 deletions tar/chunks/chunks_stats_internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "asprintf.h"
#include "humansize.h"
#include "print_separator.h"
#include "storage.h"
#include "tarsnap_opt.h"
#include "warnp.h"
Expand Down Expand Up @@ -64,26 +65,43 @@ chunks_stats_addstats(struct chunkstats * to, struct chunkstats * from)
}

/**
* chunks_stats_printheader(stream, csv):
* chunks_stats_printheader(stream, csv, print_nulls):
* Print a header line for statistics to ${stream}, optionally in ${csv}
* format.
* format. If ${print_nulls} is non-zero, use '\0' for separators.
*/
int
chunks_stats_printheader(FILE * stream, int csv)
chunks_stats_printheader(FILE * stream, int csv, int print_nulls)
{
const char * first_field;

if (csv) {
if (csv || print_nulls) {
first_field = csv ? "Archive name" : "";
if (fprintf(stream, "%s", first_field) < 0) {
warnp("fprintf");
goto err0;
}
if (print_separator(stream, ",", print_nulls, 2))
goto err0;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally we'd have

...) {
        warnp("fprintf");
        goto err0;
}

here, but we don't need the warnp when we switch to print_separator() in the following commit.

I normally try to have a "perfect" refactoring before changing code, but in this specific case I think it's worth skipping the warning in order to reduce the number of changed lines in the diff.

#ifdef STATS_WITH_CHUNKS
if (fprintf(stream, "%s,%s,%s,%s\n",
"Archive name", "# of chunks", "Total size",
"Compressed size") < 0) {
#else
if (fprintf(stream, "%s,%s,%s\n",
"Archive name", "Total size", "Compressed size") < 0) {
if (fprintf(stream, "%s", "# of chunks") < 0) {
warnp("fprintf");
goto err0;
}
if (print_separator(stream, ",", print_nulls, 2))
goto err0;
#endif
if (fprintf(stream, "%s", "Total size") < 0) {
warnp("fprintf");
goto err0;
}
if (print_separator(stream, ",", print_nulls, 2))
goto err0;
if (fprintf(stream, "%s", "Compressed size") < 0) {
warnp("fprintf");
goto err0;
}
if (print_separator(stream, "\n", print_nulls, 1))
goto err0;
} else {
#ifdef STATS_WITH_CHUNKS
if (fprintf(stream, "%-32s %12s %15s %15s\n",
Expand All @@ -106,13 +124,15 @@ chunks_stats_printheader(FILE * stream, int csv)
}

/**
* chunks_stats_print(stream, stats, name, stats_extra, csv):
* chunks_stats_print(stream, stats, name, stats_extra, csv, print_nulls):
* Print a line with ${name} and combined statistics from ${stats} and
* ${stats_extra} to ${stream}, optionally in ${csv} format.
* ${stats_extra} to ${stream}, optionally in ${csv} format. If ${print_nulls}
* is non-zero, use '\0' as separators.
*/
int
chunks_stats_print(FILE * stream, struct chunkstats * stats,
const char * name, struct chunkstats * stats_extra, int csv)
const char * name, struct chunkstats * stats_extra, int csv,
int print_nulls)
{
struct chunkstats s;
char * s_lenstr, * s_zlenstr;
Expand Down Expand Up @@ -142,19 +162,33 @@ chunks_stats_print(FILE * stream, struct chunkstats * stats,
}

/* Print output line. */
if (csv) {
if (fprintf(stream,
if (csv || print_nulls) {
if (fprintf(stream, "%s", name) < 0) {
warnp("fprintf");
goto err2;
}
if (print_separator(stream, ",", print_nulls, 2))
goto err2;
#ifdef STATS_WITH_CHUNKS
"%s,%" PRIu64 ",%s,%s\n",
name, s.nchunks,
#else
"%s,%s,%s\n",
name,
if (fprintf(stream, "%" PRIu64, s.nchunks) < 0) {
warnp("fprintf");
goto err2;
}
if (print_separator(stream, ",", print_nulls, 2))
goto err2;
#endif
s_lenstr, s_zlenstr) < 0) {
if (fprintf(stream, "%s", s_lenstr) < 0) {
warnp("fprintf");
goto err2;
}
if (print_separator(stream, ",", print_nulls, 2))
goto err2;
if (fprintf(stream, "%s", s_zlenstr) < 0) {
warnp("fprintf");
goto err2;
}
if (print_separator(stream, "\n", print_nulls, 1))
goto err2;
} else {
if (fprintf(stream,
#ifdef STATS_WITH_CHUNKS
Expand Down
10 changes: 5 additions & 5 deletions tar/chunks/chunks_write.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,21 +284,21 @@ chunks_write_printstats(FILE * stream, CHUNKS_W * C, int csv)
{

/* Print header. */
if (chunks_stats_printheader(stream, csv))
if (chunks_stats_printheader(stream, csv, 0))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same argument for -c --null-output; if we wanted to enable it, this is where we'd start.

goto err0;

/* Print the statistics we have. */
if (chunks_stats_print(stream, &C->stats_total, "All archives",
&C->stats_extra, csv))
&C->stats_extra, csv, 0))
goto err0;
if (chunks_stats_print(stream, &C->stats_unique, " (unique data)",
&C->stats_extra, csv))
&C->stats_extra, csv, 0))
goto err0;
if (chunks_stats_print(stream, &C->stats_tape, "This archive",
&C->stats_tapee, csv))
&C->stats_tapee, csv, 0))
goto err0;
if (chunks_stats_print(stream, &C->stats_new, "New data",
&C->stats_tapee, csv))
&C->stats_tapee, csv, 0))
goto err0;

/* Success! */
Expand Down
9 changes: 6 additions & 3 deletions tar/glue/tape.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,21 +125,24 @@ tarsnap_mode_print_stats(struct bsdtar *bsdtar)
goto err1;

/* Print statistics about the archive set. */
if (statstape_printglobal(d, bsdtar->option_csv_filename))
if (statstape_printglobal(d, bsdtar->option_csv_filename,
bsdtar->option_null_output))
goto err2;

if (bsdtar->ntapes == 0) {
/* User only wanted global statistics. */
} else if ((bsdtar->tapenames[0][0] == '*') &&
(bsdtar->tapenames[0][1] == '\0')) {
/* User wants statistics on all archives. */
if (statstape_printall(d, bsdtar->option_csv_filename))
if (statstape_printall(d, bsdtar->option_csv_filename,
bsdtar->option_null_output))
goto err2;
} else {
/* User wants statistics about specific archive(s). */
for (i = 0; i < bsdtar->ntapes; i++) {
switch (statstape_print(d, bsdtar->tapenames[i],
bsdtar->option_csv_filename)) {
bsdtar->option_csv_filename,
bsdtar->option_null_output)) {
case 0:
break;
case 1:
Expand Down
Loading