fix: address rsync info review findings
CI / lint (pull_request) Successful in 12s
CI / sanitizers (undefined) (pull_request) Successful in 37s
CI / sanitizers (address) (pull_request) Successful in 38s
CI / fuzz-build (pull_request) Successful in 14s
CI / coverage (pull_request) Successful in 31s
CI / build-and-test (pull_request) Successful in 1m16s
CI / valgrind (pull_request) Successful in 33s
CI / lint (pull_request) Successful in 12s
CI / sanitizers (undefined) (pull_request) Successful in 37s
CI / sanitizers (address) (pull_request) Successful in 38s
CI / fuzz-build (pull_request) Successful in 14s
CI / coverage (pull_request) Successful in 31s
CI / build-and-test (pull_request) Successful in 1m16s
CI / valgrind (pull_request) Successful in 33s
This commit is contained in:
+1
-1
@@ -23,7 +23,7 @@ This document maps rsync's full feature set to FastSync's current implementation
|
||||
| `-q`, `--quiet` | Suppress non-error messages | ❌ Not Implemented | Removed because it had no effect |
|
||||
| `--help` | Show help | ✅ Implemented | Prints usage and exits; `-h` is not accepted |
|
||||
| `-V`, `--version` | Print version | ✅ Implemented | |
|
||||
| `--info=FLAGS` | Fine-grained info verbosity | ✅ Implemented | Supports comma-separated rsync info names and `all`/`none` |
|
||||
| `--info=FLAGS` | Fine-grained info verbosity | ✅ Implemented | Supports `copy`, `misc`, `skip`, `stats`, and `none`; unsupported names are rejected |
|
||||
| `--debug=FLAGS` | Fine-grained debug verbosity | ❌ Not Implemented | Removed because it had no effect |
|
||||
| `--stderr=MODE` | Change stderr output mode | ❌ Not Implemented | |
|
||||
| `--no-motd` | Suppress daemon MOTD | ❌ Not Implemented | |
|
||||
|
||||
+14
-18
@@ -115,30 +115,14 @@ static int parse_info_flags(const char* value, Config* config) {
|
||||
}
|
||||
if (strcmp(token, "copy") == 0)
|
||||
flag = LOG_INFO_COPY;
|
||||
else if (strcmp(token, "del") == 0)
|
||||
flag = LOG_INFO_DEL;
|
||||
else if (strcmp(token, "flist") == 0)
|
||||
flag = LOG_INFO_FLIST;
|
||||
else if (strcmp(token, "misc") == 0)
|
||||
flag = LOG_INFO_MISC;
|
||||
else if (strcmp(token, "mount") == 0)
|
||||
flag = LOG_INFO_MOUNT;
|
||||
else if (strcmp(token, "name") == 0)
|
||||
flag = LOG_INFO_NAME;
|
||||
else if (strcmp(token, "nonreg") == 0)
|
||||
flag = LOG_INFO_NONREG;
|
||||
else if (strcmp(token, "progress") == 0)
|
||||
flag = LOG_INFO_PROGRESS;
|
||||
else if (strcmp(token, "skip") == 0)
|
||||
flag = LOG_INFO_SKIP;
|
||||
else if (strcmp(token, "stats") == 0)
|
||||
flag = LOG_INFO_STATS;
|
||||
else if (strcmp(token, "symsafe") == 0)
|
||||
flag = LOG_INFO_SYMSAFE;
|
||||
else if (strcmp(token, "backup") == 0)
|
||||
flag = LOG_INFO_BACKUP;
|
||||
else {
|
||||
log_message(LOG_LEVEL_ERROR, "unknown --info flag: %s", token);
|
||||
log_message(LOG_LEVEL_ERROR, "unsupported --info flag: %s", token);
|
||||
free(flags);
|
||||
return -1;
|
||||
}
|
||||
@@ -147,7 +131,6 @@ static int parse_info_flags(const char* value, Config* config) {
|
||||
free(flags);
|
||||
config->info_level = (int)parsed;
|
||||
set_log_info_flags(parsed);
|
||||
set_log_level(LOG_LEVEL_INFO);
|
||||
return 0;
|
||||
}
|
||||
|
||||
@@ -273,6 +256,19 @@ static int apply_table_option(Config* config, const OptionEntry* entry, const ch
|
||||
/* Parse CLI arguments into config. Returns 0 on success, -1 on error, 1 for help/clean-exit. */
|
||||
int parse_args(Config* config, int argc, char* argv[], int* positional_args,
|
||||
int* positional_count) {
|
||||
/* Apply output controls before processing other options so their order is irrelevant. */
|
||||
for (int i = 1; i < argc; i++) {
|
||||
if (strcmp(argv[i], "-v") == 0 || strcmp(argv[i], "--verbose") == 0) {
|
||||
set_log_level(LOG_LEVEL_DEBUG);
|
||||
} else if (strncmp(argv[i], "--info=", 7) == 0) {
|
||||
if (parse_info_flags(argv[i] + 7, config) != 0)
|
||||
return -1;
|
||||
} else if (strcmp(argv[i], "--info") == 0) {
|
||||
if (i + 1 >= argc || parse_info_flags(argv[++i], config) != 0)
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
|
||||
for (int i = 1; i < argc; i++) {
|
||||
const OptionEntry* entry = find_table_option(argv[i]);
|
||||
if (entry) {
|
||||
|
||||
@@ -422,6 +422,14 @@ static int send_chunks_multithreaded(void* pipeline_context) {
|
||||
goto send_fail;
|
||||
}
|
||||
bool ok = finalize_transfer(client);
|
||||
mtx_lock(&context->mutex_progress);
|
||||
int total_files = context->total_files;
|
||||
unsigned long long total_bytes = context->total_bytes;
|
||||
mtx_unlock(&context->mutex_progress);
|
||||
if (context->config->stats)
|
||||
fprintf(stderr, "Stats: %d files, %.1f MB\n", total_files, total_bytes / 1048576.0);
|
||||
log_info_message(LOG_INFO_STATS, "Transfer summary: %d files, %.1f MB", total_files,
|
||||
total_bytes / 1048576.0);
|
||||
disconnect_transfer_client(client);
|
||||
mark_sender_done(context);
|
||||
protocol_session_unbind();
|
||||
@@ -443,16 +451,19 @@ static int send_chunks_multithreaded(void* pipeline_context) {
|
||||
protocol_session_unbind();
|
||||
return thrd_error;
|
||||
}
|
||||
if (context->config->show_progress) {
|
||||
unsigned long long chunk_bytes = 0;
|
||||
for (int i = 0; i < current_chunk->element_count; i++) {
|
||||
if (current_chunk->items[i] && current_chunk->items[i]->data)
|
||||
chunk_bytes += current_chunk->items[i]->data->size;
|
||||
unsigned long long chunk_bytes = 0;
|
||||
int chunk_files = 0;
|
||||
for (int i = 0; i < current_chunk->element_count; i++) {
|
||||
if (current_chunk->items[i] && current_chunk->items[i]->data) {
|
||||
chunk_files++;
|
||||
chunk_bytes += current_chunk->items[i]->data->size;
|
||||
}
|
||||
mtx_lock(&context->mutex_progress);
|
||||
context->progress_bytes += chunk_bytes;
|
||||
mtx_unlock(&context->mutex_progress);
|
||||
}
|
||||
mtx_lock(&context->mutex_progress);
|
||||
context->total_files += chunk_files;
|
||||
context->total_bytes += chunk_bytes;
|
||||
context->progress_bytes = context->total_bytes;
|
||||
mtx_unlock(&context->mutex_progress);
|
||||
chunk_destroy(current_chunk);
|
||||
}
|
||||
}
|
||||
@@ -653,8 +664,8 @@ int send_files(Config* config) {
|
||||
manifest = NULL;
|
||||
break;
|
||||
}
|
||||
total_bytes += chunk_bytes;
|
||||
if (config->show_progress) {
|
||||
total_bytes += chunk_bytes;
|
||||
time_t now = time(NULL);
|
||||
if (now - last_progress >= 1) {
|
||||
last_progress = now;
|
||||
|
||||
+1
-2
@@ -37,8 +37,7 @@ void print_usage(void) {
|
||||
printf(" -s Enable chunk serialization\n");
|
||||
printf(" -f Enable sendfile (TCP only, not with -c or -s)\n");
|
||||
printf(" -v, --verbose Enable debug logging\n");
|
||||
printf(" --info=FLAGS Fine-grained info: copy,del,flist,misc,mount,name,nonreg,\n");
|
||||
printf(" progress,skip,stats,symsafe,backup,all,none\n");
|
||||
printf(" --info=FLAGS Fine-grained info: copy,misc,skip,stats,all,none\n");
|
||||
printf(" -M, --preserve Preserve file metadata\n");
|
||||
printf(" --chunk-size <n> Chunk size in bytes (default: %d)\n", DEFAULT_CHUNK_SIZE);
|
||||
printf(" --source-dir <path> Source directory\n");
|
||||
|
||||
+4
-12
@@ -8,18 +8,10 @@ typedef enum { LOG_LEVEL_DEBUG, LOG_LEVEL_INFO, LOG_LEVEL_WARNING, LOG_LEVEL_ERR
|
||||
|
||||
typedef enum {
|
||||
LOG_INFO_COPY = 1u << 0,
|
||||
LOG_INFO_DEL = 1u << 1,
|
||||
LOG_INFO_FLIST = 1u << 2,
|
||||
LOG_INFO_MISC = 1u << 3,
|
||||
LOG_INFO_MOUNT = 1u << 4,
|
||||
LOG_INFO_NAME = 1u << 5,
|
||||
LOG_INFO_NONREG = 1u << 6,
|
||||
LOG_INFO_PROGRESS = 1u << 7,
|
||||
LOG_INFO_SKIP = 1u << 8,
|
||||
LOG_INFO_STATS = 1u << 9,
|
||||
LOG_INFO_SYMSAFE = 1u << 10,
|
||||
LOG_INFO_BACKUP = 1u << 11,
|
||||
LOG_INFO_ALL = (1u << 12) - 1,
|
||||
LOG_INFO_MISC = 1u << 1,
|
||||
LOG_INFO_SKIP = 1u << 2,
|
||||
LOG_INFO_STATS = 1u << 3,
|
||||
LOG_INFO_ALL = LOG_INFO_COPY | LOG_INFO_MISC | LOG_INFO_SKIP | LOG_INFO_STATS,
|
||||
} LogInfoFlag;
|
||||
|
||||
void log_message(LogLevel log_level, const char* message, ...);
|
||||
|
||||
@@ -26,7 +26,9 @@ PipelineContextSender* pipeline_context_sender_create(Config* config, Queue* que
|
||||
context->scanner_done = false;
|
||||
context->loader_done = false;
|
||||
context->manifest = NULL;
|
||||
context->total_files = 0;
|
||||
context->progress_bytes = 0;
|
||||
context->total_bytes = 0;
|
||||
context->sender_done = false;
|
||||
atomic_init(&context->cancelled, false);
|
||||
int init = 0;
|
||||
|
||||
@@ -25,7 +25,9 @@ typedef struct {
|
||||
bool loader_done;
|
||||
ArrayList* manifest;
|
||||
mtx_t mutex_progress;
|
||||
int total_files;
|
||||
unsigned long long progress_bytes;
|
||||
unsigned long long total_bytes;
|
||||
bool sender_done;
|
||||
atomic_bool cancelled;
|
||||
} PipelineContextSender;
|
||||
|
||||
@@ -276,13 +276,24 @@ class TestInfo:
|
||||
output = result.stdout + result.stderr
|
||||
assert "[INFO]" in output and "Transferring" in output
|
||||
|
||||
def test_info_stats_reports_multithreaded_transfer(self, shared_server):
|
||||
clean_dir(DEST_DIR)
|
||||
result, _ = run_client(
|
||||
SOURCE_DIR, DEST_DIR,
|
||||
flags=["-m", "--info=stats"],
|
||||
port=shared_server.port,
|
||||
)
|
||||
assert result.returncode == 0, f"Info stats sync failed: {(result.stderr or result.stdout)[:200]}"
|
||||
output = result.stdout + result.stderr
|
||||
assert "[INFO]" in output and "Transfer summary:" in output
|
||||
|
||||
def test_info_rejects_unknown_flag(self):
|
||||
result, _ = run_client(
|
||||
SOURCE_DIR, DEST_DIR,
|
||||
flags=["--info=unknown"],
|
||||
)
|
||||
assert result.returncode != 0
|
||||
assert "unknown --info flag" in result.stderr
|
||||
assert "unsupported --info flag" in result.stderr
|
||||
|
||||
|
||||
class TestBandwidthLimit:
|
||||
|
||||
@@ -335,6 +335,17 @@ static void test_parse_args_info_flags() {
|
||||
config_delete(cfg);
|
||||
}
|
||||
|
||||
static void test_parse_args_info_verbose_order() {
|
||||
Config* cfg = config_create();
|
||||
char* argv[] = {"fastsync", "--info=copy", "--verbose", "/src", "/dst"};
|
||||
int positional_args[2];
|
||||
int positional_count = 0;
|
||||
|
||||
EXPECT_EQ_INT(parse_args(cfg, 5, argv, positional_args, &positional_count), 0);
|
||||
EXPECT_EQ_INT(get_log_info_flags(), LOG_INFO_COPY);
|
||||
config_delete(cfg);
|
||||
}
|
||||
|
||||
static void test_parse_args_rejects_invalid_info_flag() {
|
||||
Config* cfg = config_create();
|
||||
char* argv[] = {"fastsync", "--info=copy,unknown", "/src", "/dst"};
|
||||
@@ -382,6 +393,7 @@ void test_client_cli() {
|
||||
test_parse_args_unknown_option();
|
||||
test_parse_args_rejects_unimplemented_options();
|
||||
test_parse_args_info_flags();
|
||||
test_parse_args_info_verbose_order();
|
||||
test_parse_args_rejects_invalid_info_flag();
|
||||
test_parse_args_archive();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user