)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"1c3e74328ea2bb794963539ad3277e33805fafd8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"0bd1906b_6e06365e","updated":"2023-12-11 21:52:30.000000000","message":"Mostly nitpicks, I would like the docs on the lookup methods to be clarified\nbecause it can be hard to read on first read.","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"}],"tvix/glue/src/tvix_build.rs":[{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"1c3e74328ea2bb794963539ad3277e33805fafd8","unresolved":true,"context_lines":[{"line_number":29,"context_line":"];"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"/// Takes a [Derivation] and turns it into a [BuildRequest]."},{"line_number":32,"context_line":"/// It assumes the Derivation has been validated."},{"line_number":33,"context_line":"/// It needs two lookup functions:"},{"line_number":34,"context_line":"/// - one translating input sources to a castore node"},{"line_number":35,"context_line":"/// - one translating input derivations and (a subset of their) output names to"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"dd921a18_69954141","line":32,"updated":"2023-12-11 21:52:30.000000000","message":"nit: IMHO, `debug_assert!(drv.validated())`.","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"b5cf4d0a98634edca24190fd7e93c5062280680d","unresolved":false,"context_lines":[{"line_number":29,"context_line":"];"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"/// Takes a [Derivation] and turns it into a [BuildRequest]."},{"line_number":32,"context_line":"/// It assumes the Derivation has been validated."},{"line_number":33,"context_line":"/// It needs two lookup functions:"},{"line_number":34,"context_line":"/// - one translating input sources to a castore node"},{"line_number":35,"context_line":"/// - one translating input derivations and (a subset of their) output names to"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"66806bde_dc39d4f1","line":32,"in_reply_to":"dd921a18_69954141","updated":"2023-12-11 22:29:14.000000000","message":"Done","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"1c3e74328ea2bb794963539ad3277e33805fafd8","unresolved":true,"context_lines":[{"line_number":31,"context_line":"/// Takes a [Derivation] and turns it into a [BuildRequest]."},{"line_number":32,"context_line":"/// It assumes the Derivation has been validated."},{"line_number":33,"context_line":"/// It needs two lookup functions:"},{"line_number":34,"context_line":"/// - one translating input sources to a castore node"},{"line_number":35,"context_line":"/// - one translating input derivations and (a subset of their) output names to"},{"line_number":36,"context_line":"///   a castore node."},{"line_number":37,"context_line":"/// TODO: add NIX_BUILD_CORES?"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"a1db3cbb_d471e74b","line":34,"updated":"2023-12-11 21:52:30.000000000","message":"that is `fn_input_sources_to_node`","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"b5cf4d0a98634edca24190fd7e93c5062280680d","unresolved":false,"context_lines":[{"line_number":31,"context_line":"/// Takes a [Derivation] and turns it into a [BuildRequest]."},{"line_number":32,"context_line":"/// It assumes the Derivation has been validated."},{"line_number":33,"context_line":"/// It needs two lookup functions:"},{"line_number":34,"context_line":"/// - one translating input sources to a castore node"},{"line_number":35,"context_line":"/// - one translating input derivations and (a subset of their) output names to"},{"line_number":36,"context_line":"///   a castore node."},{"line_number":37,"context_line":"/// TODO: add NIX_BUILD_CORES?"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"6d6559e1_1cf17c4f","line":34,"in_reply_to":"a1db3cbb_d471e74b","updated":"2023-12-11 22:29:14.000000000","message":"Done","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"1c3e74328ea2bb794963539ad3277e33805fafd8","unresolved":true,"context_lines":[{"line_number":32,"context_line":"/// It assumes the Derivation has been validated."},{"line_number":33,"context_line":"/// It needs two lookup functions:"},{"line_number":34,"context_line":"/// - one translating input sources to a castore node"},{"line_number":35,"context_line":"/// - one translating input derivations and (a subset of their) output names to"},{"line_number":36,"context_line":"///   a castore node."},{"line_number":37,"context_line":"/// TODO: add NIX_BUILD_CORES?"},{"line_number":38,"context_line":"#[allow(dead_code)]"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"be241ed1_794ed506","line":35,"updated":"2023-12-11 21:52:30.000000000","message":"that is `fn_input_drvs_to_node`","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"b5cf4d0a98634edca24190fd7e93c5062280680d","unresolved":false,"context_lines":[{"line_number":32,"context_line":"/// It assumes the Derivation has been validated."},{"line_number":33,"context_line":"/// It needs two lookup functions:"},{"line_number":34,"context_line":"/// - one translating input sources to a castore node"},{"line_number":35,"context_line":"/// - one translating input derivations and (a subset of their) output names to"},{"line_number":36,"context_line":"///   a castore node."},{"line_number":37,"context_line":"/// TODO: add NIX_BUILD_CORES?"},{"line_number":38,"context_line":"#[allow(dead_code)]"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"024c1de9_dfbb7532","line":35,"in_reply_to":"be241ed1_794ed506","updated":"2023-12-11 22:29:14.000000000","message":"Done","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"1c3e74328ea2bb794963539ad3277e33805fafd8","unresolved":true,"context_lines":[{"line_number":34,"context_line":"/// - one translating input sources to a castore node"},{"line_number":35,"context_line":"/// - one translating input derivations and (a subset of their) output names to"},{"line_number":36,"context_line":"///   a castore node."},{"line_number":37,"context_line":"/// TODO: add NIX_BUILD_CORES?"},{"line_number":38,"context_line":"#[allow(dead_code)]"},{"line_number":39,"context_line":"fn derivation_to_build_request\u003cFIS, FID\u003e("},{"line_number":40,"context_line":"    derivation: \u0026Derivation,"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"1c748b14_40aa3a7e","line":37,"updated":"2023-12-11 21:52:30.000000000","message":"future: Surely, we should take the time to pause and think about it.\nHow do we envision a system where we schedule appropriately the tasks and optimize the cores on a given set of machines?","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"b5cf4d0a98634edca24190fd7e93c5062280680d","unresolved":false,"context_lines":[{"line_number":34,"context_line":"/// - one translating input sources to a castore node"},{"line_number":35,"context_line":"/// - one translating input derivations and (a subset of their) output names to"},{"line_number":36,"context_line":"///   a castore node."},{"line_number":37,"context_line":"/// TODO: add NIX_BUILD_CORES?"},{"line_number":38,"context_line":"#[allow(dead_code)]"},{"line_number":39,"context_line":"fn derivation_to_build_request\u003cFIS, FID\u003e("},{"line_number":40,"context_line":"    derivation: \u0026Derivation,"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"93b528e4_68e26876","line":37,"in_reply_to":"1c748b14_40aa3a7e","updated":"2023-12-11 22:29:14.000000000","message":"I think there might be some oracle determining how many cores to use, based on previous history.\n\nThere\u0027s also the distinction between how many cores we make available to the build, and how we set this variable too. Ideally, the builder wouldn\u0027t need to understand the meaning of NIX_BUILD_CORES, our stdenv would set NIX_BUILD_CORES on its own after consulting `$(nproc)` and we\u0027d be done.\n\nThere\u0027s also a certain charme in being able to identify BuildRequest by content-addressing them, having this NIX_BUILD_CORES leak in there is ugly.\n\nI want to leave this for later, once we actually start some builds.\n\nI removed that specific TODO, the one in L18 still is there.","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"1c3e74328ea2bb794963539ad3277e33805fafd8","unresolved":true,"context_lines":[{"line_number":118,"context_line":"    }"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    // validate all nodes are actually Some."},{"line_number":121,"context_line":"    for input in inputs.iter() {"},{"line_number":122,"context_line":"        debug_assert!(input.node.is_some());"},{"line_number":123,"context_line":"    }"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"dae4cedc_ac557dcb","line":121,"updated":"2023-12-11 21:52:30.000000000","message":"nit: sounds like `debug_assert!(inputs.iter().all(|inode| inode.is_some())`","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"b5cf4d0a98634edca24190fd7e93c5062280680d","unresolved":false,"context_lines":[{"line_number":118,"context_line":"    }"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    // validate all nodes are actually Some."},{"line_number":121,"context_line":"    for input in inputs.iter() {"},{"line_number":122,"context_line":"        debug_assert!(input.node.is_some());"},{"line_number":123,"context_line":"    }"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"f9cad330_d7929bfa","line":121,"in_reply_to":"dae4cedc_ac557dcb","updated":"2023-12-11 22:29:14.000000000","message":"Done","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"1c3e74328ea2bb794963539ad3277e33805fafd8","unresolved":true,"context_lines":[{"line_number":123,"context_line":"    }"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    // sort inputs by their name"},{"line_number":126,"context_line":"    // TODO: can we avoid cloning here?"},{"line_number":127,"context_line":"    inputs.sort_unstable_by_key(|e| {"},{"line_number":128,"context_line":"        e.node"},{"line_number":129,"context_line":"            .as_ref()"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"35d6e739_62ecbe69","line":126,"updated":"2023-12-11 21:52:30.000000000","message":"you would need a custom compare function for reference of your structure imho\nwhich would do `*x \u003d\u003d *y` and this would work well.","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"b5cf4d0a98634edca24190fd7e93c5062280680d","unresolved":true,"context_lines":[{"line_number":123,"context_line":"    }"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    // sort inputs by their name"},{"line_number":126,"context_line":"    // TODO: can we avoid cloning here?"},{"line_number":127,"context_line":"    inputs.sort_unstable_by_key(|e| {"},{"line_number":128,"context_line":"        e.node"},{"line_number":129,"context_line":"            .as_ref()"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"e82db434_3e5b7d6f","line":126,"in_reply_to":"35d6e739_62ecbe69","updated":"2023-12-11 22:29:14.000000000","message":"inputs.sort_by with a custom comparator did work without any cloning.","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"20d2176da37f5a6b925292c2a42a81f350a0b13e","unresolved":false,"context_lines":[{"line_number":123,"context_line":"    }"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    // sort inputs by their name"},{"line_number":126,"context_line":"    // TODO: can we avoid cloning here?"},{"line_number":127,"context_line":"    inputs.sort_unstable_by_key(|e| {"},{"line_number":128,"context_line":"        e.node"},{"line_number":129,"context_line":"            .as_ref()"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"4c45387a_2006bc23","line":126,"in_reply_to":"e82db434_3e5b7d6f","updated":"2023-12-11 22:35:30.000000000","message":"Acknowledged","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"1c3e74328ea2bb794963539ad3277e33805fafd8","unresolved":true,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    // sort inputs by their name"},{"line_number":126,"context_line":"    // TODO: can we avoid cloning here?"},{"line_number":127,"context_line":"    inputs.sort_unstable_by_key(|e| {"},{"line_number":128,"context_line":"        e.node"},{"line_number":129,"context_line":"            .as_ref()"},{"line_number":130,"context_line":"            .expect(\"node must be some\")"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"6a7998ec_98c2b59a","line":127,"updated":"2023-12-11 21:52:30.000000000","message":"this is going to be slower than sort_cached_by_key given that you allocate all the time in the key: https://doc.rust-lang.org/std/primitive.slice.html#method.sort_by_cached_key","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"b5cf4d0a98634edca24190fd7e93c5062280680d","unresolved":false,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    // sort inputs by their name"},{"line_number":126,"context_line":"    // TODO: can we avoid cloning here?"},{"line_number":127,"context_line":"    inputs.sort_unstable_by_key(|e| {"},{"line_number":128,"context_line":"        e.node"},{"line_number":129,"context_line":"            .as_ref()"},{"line_number":130,"context_line":"            .expect(\"node must be some\")"}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"d9e53bdb_d9586434","line":127,"in_reply_to":"6a7998ec_98c2b59a","updated":"2023-12-11 22:29:14.000000000","message":"inputs.sort_by with a custom comparator did work without any cloning.","commit_id":"8edb153e5616cdeda0e56c53817c515a88941e62"}]}