Add PG header tag to samtools import.#2008
Conversation
| if (!opts->no_pg) { | ||
| char *arg_list; | ||
| if (!(arg_list = stringify_argv(argc+2, argv-2))) { | ||
| print_error("view", "failed to create arg_list"); | ||
| goto err; | ||
| } | ||
| if (sam_hdr_add_pg(hdr_out, "samtools", | ||
| "VN", samtools_version(), | ||
| arg_list ? "CL" : NULL, | ||
| arg_list ? arg_list : NULL, | ||
| NULL)) { | ||
| fprintf(stderr, "Failed to add PG line to the header"); | ||
| free(arg_list); | ||
| goto err; | ||
| } | ||
|
|
||
| free(arg_list); | ||
| } |
There was a problem hiding this comment.
Only the last option is being written on the PG line. Most of the other stringify uses in samtools use stringify_argv(argc+1, argv-1)
There was a problem hiding this comment.
I initially copied what was being done elsewhere and it wasn't including the bit I wanted so this was the fix.
Maybe I got something wrong or was comparing to the wrong thing. I'll check again. Thanks
There was a problem hiding this comment.
Doh - it corrected it for my one naive test, but not generally. The problem is argc/argv has been modified already to be the end of the argument list rather than the start. See https://github.com/samtools/samtools/blob/develop/bam_import.c#L514
It needs a slight function change to correct this.
There was a problem hiding this comment.
Or not as optind is a global so I can use that to undo the other change for purposes of CL printing. I squashed in this change:
diff --git a/bam_import.c b/bam_import.c
index 217a046d..452ce4dc 100644
--- a/bam_import.c
+++ b/bam_import.c
@@ -262,7 +262,7 @@ static int import_fastq(int argc, char **argv, opts_t *opts) {
if (!opts->no_pg) {
char *arg_list;
- if (!(arg_list = stringify_argv(argc+2, argv-2))) {
+ if (!(arg_list = stringify_argv(argc+1+optind, argv-1-optind))) {
print_error("view", "failed to create arg_list");
goto err;
}
The --no-PG command line argument was already there, but the code to check it was absent. (Reported by Steven Leonard)
The --no-PG command line argument was already there, but the code to check it was absent. (Reported by Steven Leonard)