fix: include sources referenced but not embedded by an object file

src/commands/debug_files/bundle_sources.rs

@@ -87,10 +87,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
 
         for (index, object) in archive.get().objects().enumerate() {
             let object = object?;
-            if object.has_sources() {
-                eprintln!("skipped {orig_path} (no source info)");
-                continue;
-            }
 
             let mut out = output_path.unwrap_or(parent_path).join(filename);
             match index {
@@ -107,7 +103,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
             let written = writer.write_object_with_filter(
                 &object,
                 &filename.to_string_lossy(),
-                |file, _source_descriptor| filter_bad_sources(file),
+                filter_bad_sources,
             )?;
 
             if !written {

src/commands/debug_files/print_sources.rs

@@ -1,3 +1,4 @@
+use std::fmt;
 use std::path::Path;
 
 use anyhow::Result;
@@ -67,9 +68,15 @@ fn print_object_sources(object: &Object) -> Result<()> {
             let abs_path = file.abs_path_str();
             println!("  {}", &abs_path);
             let source = debug_session.source_by_path(abs_path.as_str())?;
-            match source.as_ref().and_then(|sd| sd.contents()) {
+            match source {
                 Some(source) => {
-                    println!("    Embedded, {} bytes", source.len());
+                    print_file_descriptor_detail(
+                        "Embedded",
+                        source.contents().map(|v| format!("{} bytes", v.len())),
+                    );
+                    print_file_descriptor_detail("Url", source.url());
+                    print_file_descriptor_detail("DebugId", source.debug_id());
+                    print_file_descriptor_detail("SourceMap Url", source.source_mapping_url());
                 }
                 None => {
                     if Path::new(&abs_path).exists() {
@@ -83,3 +90,9 @@ fn print_object_sources(object: &Object) -> Result<()> {
     }
     Ok(())
 }
+
+fn print_file_descriptor_detail<T: fmt::Display>(name: &str, value: Option<T>) {
+    if let Some(value) = value {
+        println!("    {name}: {value}");
+    }
+}

src/utils/dif_upload.rs

@@ -25,7 +25,7 @@ use sha1_smol::Digest;
 use symbolic::common::{AsSelf, ByteView, DebugId, SelfCell, Uuid};
 use symbolic::debuginfo::macho::{BcSymbolMap, UuidMapping};
 use symbolic::debuginfo::pe::PeObject;
-use symbolic::debuginfo::sourcebundle::SourceBundleWriter;
+use symbolic::debuginfo::sourcebundle::{SourceBundleWriter, SourceFileDescriptor};
 use symbolic::debuginfo::{Archive, FileEntry, FileFormat, Object};
 use symbolic::il2cpp::ObjectLineMapping;
 use walkdir::WalkDir;
@@ -1104,16 +1104,27 @@ fn extract_embedded_ppdb<'a>(pe: &PeObject, pe_name: &str) -> Result<Option<DifM
 }
 
 /// Default filter function to skip over bad sources we do not want to include.
-pub fn filter_bad_sources(entry: &FileEntry) -> bool {
+pub fn filter_bad_sources(
+    entry: &FileEntry,
+    embedded_source: &Option<SourceFileDescriptor>,
+) -> bool {
     let max_size = Config::current().get_max_dif_item_size();
     let path = &entry.abs_path_str();
 
-    if entry.name_str().ends_with(".pch") {
-        // always ignore pch files
+    // Ignore pch files.
+    if path.ends_with(".pch") {
         return false;
-    } else if let Ok(meta) = fs::metadata(path) {
+    }
+
+    // Ignore files embedded in the object itself.
+    if embedded_source.is_some() {
+        debug!("Skipping embedded source file: {}", path);
+        return false;
+    }
+
+    // Ignore files larger than limit (defaults to `DEFAULT_MAX_DIF_ITEM_SIZE`).
+    if let Ok(meta) = fs::metadata(path) {
         let item_size = meta.len();
-        // ignore files larger than limit (defaults to 1MB)
         if item_size > max_size {
             warn!(
                 "Source exceeded maximum item size limit ({}). {}",
@@ -1124,6 +1135,7 @@ pub fn filter_bad_sources(entry: &FileEntry) -> bool {
     }
 
     // if a file metadata could not be read it will be skipped later.
+    debug!("Trying to add source file: {}", path);
     true
 }
 
@@ -1156,12 +1168,6 @@ fn create_source_bundles<'a>(
             Some(object) => object,
             None => continue,
         };
-        if object.has_sources() {
-            // Do not create standalone source bundles if the original object already contains
-            // source code. This would just store duplicate information in Sentry.
-            debug!("skipping {} because it already embeds sources", name);
-            continue;
-        }
 
         let temp_file = TempFile::create()?;
         let mut writer = SourceBundleWriter::start(BufWriter::new(temp_file.open()?))?;
@@ -1170,11 +1176,8 @@ fn create_source_bundles<'a>(
         // Resolve source files from the object and write their contents into the archive. Skip to
         // upload this bundle if no source could be written. This can happen if there is no file or
         // line information in the object file, or if none of the files could be resolved.
-        let written = writer.write_object_with_filter(
-            object,
-            dif.file_name(),
-            |file, _source_descriptor| filter_bad_sources(file),
-        )?;
+        let written =
+            writer.write_object_with_filter(object, dif.file_name(), filter_bad_sources)?;
         if !written {
             debug!("No sources found for {}", name);
             continue;

tests/integration/_cases/debug_files/debug_files-bundle_sources-mixed-embedded-sources.trycmd

@@ -0,0 +1,14 @@
+```
+$ sentry-cli --log-level=debug debug-files bundle-sources tests/integration/_fixtures/SrcGenSampleApp.pdb
+? success
+  DEBUG   [..] sentry-cli version: [VERSION], platform: "[..]", architecture: "[..]"
+  INFO    [..] sentry-cli was invoked with the following command line: "[CWD]/target/debug/sentry-cli[EXE]" "--log-level=debug" "debug-files" "bundle-sources" "tests/integration/_fixtures/SrcGenSampleApp.pdb"
+  DEBUG   [..] Trying to add source file: /Users/matt/Code/temp/SrcGenSampleApp/SrcGenSampleApp/Program.cs
+  DEBUG   [..] Trying to add source file: /Users/matt/Code/temp/SrcGenSampleApp/SrcGenSampleApp/obj/Release/net6.0/SrcGenSampleApp.GlobalUsings.g.cs
+  DEBUG   [..] Trying to add source file: /Users/matt/Code/temp/SrcGenSampleApp/SrcGenSampleApp/obj/Release/net6.0/.NETCoreApp,Version=v6.0.AssemblyAttributes.cs
+  DEBUG   [..] Trying to add source file: /Users/matt/Code/temp/SrcGenSampleApp/SrcGenSampleApp/obj/Release/net6.0/SrcGenSampleApp.AssemblyInfo.cs
+  DEBUG   [..] Skipping embedded source file: /Users/matt/Code/temp/SrcGenSampleApp/SrcGenSampleApp/MySourceGenerator/MySourceGenerator.HelloSourceGenerator/Program.g.cs
+skipped tests/integration/_fixtures/SrcGenSampleApp.pdb (no files found)
+  DEBUG   [..] skipping update nagger because session is not attended
+
+```

tests/integration/_cases/debug_files/debug_files-print_sources-dll-embedded-ppdb-with-sources.trycmd

@@ -4,14 +4,14 @@ $ sentry-cli debug-files print-sources tests/integration/_fixtures/Sentry.Sample
 pe 623535c7-c0ea-4dee-b99b-4669e99a7ecc-a878d1fa has no sources.
 portablepdb 623535c7-c0ea-4dee-b99b-4669e99a7ecc-a878d1fa references sources:
   C:/dev/sentry-dotnet/samples/Sentry.Samples.Console.Basic/Program.cs
-    Embedded, 204 bytes
+    Embedded: 204 bytes
   C:/dev/sentry-dotnet/samples/Sentry.Samples.Console.Basic/obj/Release/net6.0/Sentry.Samples.Console.Basic.GlobalUsings.g.cs
-    Embedded, 295 bytes
+    Embedded: 295 bytes
   C:/dev/sentry-dotnet/samples/Sentry.Samples.Console.Basic/obj/Release/net6.0/.NETCoreApp,Version=v6.0.AssemblyAttributes.cs
-    Embedded, 198 bytes
+    Embedded: 198 bytes
   C:/dev/sentry-dotnet/samples/Sentry.Samples.Console.Basic/obj/Release/net6.0/Sentry.Attributes.cs
-    Embedded, 610 bytes
+    Embedded: 610 bytes
   C:/dev/sentry-dotnet/samples/Sentry.Samples.Console.Basic/obj/Release/net6.0/Sentry.Samples.Console.Basic.AssemblyInfo.cs
-    Embedded, 1019 bytes
+    Embedded: 1019 bytes
 
 ```

tests/integration/_cases/debug_files/debug_files-print_sources-mixed-embedded-sources.trycmd

@@ -0,0 +1,16 @@
+```
+$ sentry-cli debug-files print-sources tests/integration/_fixtures/SrcGenSampleApp.pdb
+? success
+portablepdb c02651ae-cd6f-492d-bc33-0b83111e7106-8d8e7c60 references sources:
+  /Users/matt/Code/temp/SrcGenSampleApp/SrcGenSampleApp/Program.cs
+    Not embedded nor available locally at the referenced path.
+  /Users/matt/Code/temp/SrcGenSampleApp/SrcGenSampleApp/obj/Release/net6.0/SrcGenSampleApp.GlobalUsings.g.cs
+    Not embedded nor available locally at the referenced path.
+  /Users/matt/Code/temp/SrcGenSampleApp/SrcGenSampleApp/obj/Release/net6.0/.NETCoreApp,Version=v6.0.AssemblyAttributes.cs
+    Not embedded nor available locally at the referenced path.
+  /Users/matt/Code/temp/SrcGenSampleApp/SrcGenSampleApp/obj/Release/net6.0/SrcGenSampleApp.AssemblyInfo.cs
+    Not embedded nor available locally at the referenced path.
+  /Users/matt/Code/temp/SrcGenSampleApp/SrcGenSampleApp/MySourceGenerator/MySourceGenerator.HelloSourceGenerator/Program.g.cs
+    Embedded: 243 bytes
+
+```

tests/integration/_cases/debug_files/debug_files-upload-mixed-embedded-sources.trycmd

@@ -0,0 +1,12 @@
+```
+$ sentry-cli --log-level=debug debug-files upload --include-sources tests/integration/_fixtures/SrcGenSampleApp.pdb
+? success
+  DEBUG   [..] sentry-cli version: [VERSION], platform: "[..]", architecture: "[..]"
+  INFO    [..] sentry-cli was invoked with the following command line: "[CWD]/target/debug/sentry-cli[EXE]" "--log-level=debug" "debug-files" "upload" "--include-sources" "tests/integration/_fixtures/SrcGenSampleApp.pdb"
+...
+> Found 1 debug information file (1 with embedded sources)
+> Resolved source code for 0 debug information files
+> Prepared debug information file for upload
+...
+
+```

tests/integration/debug_files/bundle_sources.rs

@@ -0,0 +1,6 @@
+use crate::integration::register_test;
+
+#[test]
+fn command_debug_files_bundle_sources_mixed_embedded_sources() {
+    register_test("debug_files/debug_files-bundle_sources-mixed-embedded-sources.trycmd");
+}

tests/integration/debug_files/mod.rs

@@ -1,5 +1,6 @@
 use crate::integration::register_test;
 
+mod bundle_sources;
 mod check;
 mod print_sources;
 mod upload;

tests/integration/debug_files/print_sources.rs

@@ -9,3 +9,8 @@ fn command_debug_files_print_sources_ppdb() {
 fn command_debug_files_print_sources_dll_embedded_ppdb_with_sources() {
     register_test("debug_files/debug_files-print_sources-dll-embedded-ppdb-with-sources.trycmd");
 }
+
+#[test]
+fn command_debug_files_print_sources_mixed_embedded_sources() {
+    register_test("debug_files/debug_files-print_sources-mixed-embedded-sources.trycmd");
+}

tests/integration/debug_files/upload.rs

@@ -131,6 +131,42 @@ fn command_debug_files_upload_dll_embedded_ppdb_with_sources() {
     register_test("debug_files/debug_files-upload-dll-embedded-ppdb-with-sources.trycmd");
 }
 
+#[test]
+fn command_debug_files_upload_mixed_embedded_sources() {
+    let _chunk_upload = mock_endpoint(
+        EndpointOptions::new("GET", "/api/0/organizations/wat-org/chunk-upload/", 200)
+            .with_response_file("debug_files/get-chunk-upload.json"),
+    );
+    let _assemble = mock_endpoint(
+        EndpointOptions::new(
+            "POST",
+            "/api/0/projects/wat-org/wat-project/files/difs/assemble/",
+            200,
+        )
+        .with_response_body(
+            r#"{
+                "21b76b717dbbd8c89e42d92b29667ac87aa3c124": {
+                    "state": "ok",
+                    "missingChunks": []
+                }
+            }"#,
+        ),
+    );
+    let _reprocessing = mock_endpoint(
+        EndpointOptions::new(
+            "POST",
+            "/api/0/projects/wat-org/wat-project/reprocessing/",
+            200,
+        )
+        .with_response_body("[]"),
+    );
+    // TODO this isn't tested properly at the moment, because `indicatif` ProgressBar (at least at the current version)
+    //      swallows debug logs printed while the progress bar is active and the session is not attended.
+    //      See how it's supposed to look like `debug_files-bundle_sources-mixed-embedded-sources.trycmd` and try it out
+    //      after an update of `indicatif` to the latest version (currently it's blocked by some other issues).
+    register_test("debug_files/debug_files-upload-mixed-embedded-sources.trycmd");
+}
+
 #[test]
 fn command_debug_files_upload_no_upload() {
     let _chunk_upload = mock_endpoint(