Ability to specify the output name for a bin target different from the crate name #9627

whereistejas · 2021-06-26T12:29:47Z

Hi,

I have opened this to close the following issue #1706 .

I have decided to start by writing a test to outline what behavior is expected from Cargo.
As of now, this test fails (for obvious reasons). I will now start writing the code needed to pass this test.

This is my first time contributing to cargo. Please, feel free to let me know if there are any protocols/processes that need to be followed. I'm a newbie at this.

Closes issue #1706

Tracking issue #9778

rust-highfive · 2021-06-26T12:29:50Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Eh2406 (or someone else) soon.

Please see the contribution instructions for more information.

alexcrichton · 2021-06-29T14:43:26Z

Thanks for the PR, and looks reasonable so far! I think we'll also want documentation updates for Cargo as well along the lines of this, and we'll probably also want this to be gated and unstable to start out, but otherwise it's a good start :)

whereistejas · 2021-07-03T16:32:18Z

Hi @alexcrichton,
I have reached a point in the code, where I could use some review. In my current implementation, I have added a new field to the TargetInner structure called binary_name to store the filename of binary provided in the Cargo.toml. The alternative approach is to store that filename directly in the name field of the TargetInner struct. But, I have no idea what kind of effect that would have on future enhancements, and it doesn't seem like a good idea to mess with something that is already working.
Do you think my approach is correct?

I have also tried the second approach here: 5a14091

It is much more elegant, I think.

whereistejas · 2021-07-08T13:13:48Z

Hi @alexcrichton / @Eh2406,
Can I please get a code review, here?

Eh2406 · 2021-07-08T13:57:03Z

I don't know this part of the code all that well. Sorry. @eric Huss may have more opinions.

ehuss · 2021-07-08T22:07:42Z

I think the approach of adding to TargetInner looks correct here.

There are a few other considerations that should be handled:

Fingerprinting needs to be able to check the timestamp of the output filename. Check that running cargo build -v a second time prints "Fresh" (that is, it doesn't build again).
cargo clean -p foo should be able to clean the binary output files.
cargo build --message-format=json should emit correct paths for artifact messages.
dep info files need to have the correct name and paths inside.

One way to get these to work is correctly add the new path to the output tracking. The way Cargo predicts what the output filenames are is kinda complicated, but I'll try to walk through it:

For each run of the compiler, Cargo has a list of OutputFile values that describe the files that rustc generates.
These are stored in CompilationFiles which in turn is part of Context.
The outputs are computed in calc_outputs_rustc.
- The style of outputs is driven by querying rustc. This is fetched in file_types which uses various caching to make that quick. Essentially it runs rustc --print=file-names … to understand the file format of each crate type.

Somewhere in that, it should compute the OutputFile with the new filename. I'm not sure where that would go, but hopefully those are some clues where to start looking.

whereistejas · 2021-07-11T19:45:52Z

src/cargo/core/compiler/build_context/target_info.rs

        }
    }

    /// The filename for this FileType that Cargo should use when "uplifting"
    /// it to the destination directory.
    pub fn uplift_filename(&self, target: &Target) -> String {
+        // for binary crate type, `should_replace_hyphens` will always be false
        let name = if self.should_replace_hyphens {


Is my assumption that should_replace_hyphens will always be false for binary crates, correct?

I think so, though this seems a little subtle. Can you test on Windows to see the behavior with PDB files works correctly in the visual studio debugger? There is a brief explanation here as to how that works.

I would agree that this is a bit subtle and I think it'd be best if this were handled a bit differently where there was an early-return or similar

Can you give me a little more background and clues on this? TBH, I'm not a Rust expert (yet). Could you elaborate what you mean by early return ?

I think to make it clearer, this could check if get_binary_name returns a value, and if it does, use that, then check should_replace_hyphens, and then the final else would return target.name(). That should make it more explicit.

whereistejas · 2021-07-11T20:09:58Z

Hi @ehuss ,
If, my understanding is correct, the filenames from OutputFile are not being supplied to rustc. Do we need to find a way to add the path from OutputFile to the rustc args ?

whereistejas · 2021-07-12T16:15:07Z

Hi,
I have used the filename from cargo.toml only for hardlinking. This satisfies all the requirements that @ehuss mentioned. The current code produces a folder structure like this:

I think we can start implementing more tests, now. One for cargo clean and one for deps. Would you like me to add them in their respective test modules in testsuites/ ?

ehuss · 2021-07-13T14:08:12Z

src/cargo/core/compiler/build_context/target_info.rs

        }
    }

    /// The filename for this FileType that Cargo should use when "uplifting"
    /// it to the destination directory.
    pub fn uplift_filename(&self, target: &Target) -> String {
+        // for binary crate type, `should_replace_hyphens` will always be false
        let name = if self.should_replace_hyphens {


I think so, though this seems a little subtle. Can you test on Windows to see the behavior with PDB files works correctly in the visual studio debugger? There is a brief explanation here as to how that works.

src/cargo/core/compiler/context/compilation_files.rs

src/cargo/core/compiler/mod.rs

whereistejas · 2021-07-13T15:19:38Z

Hi @ehuss, can we try running the CI again?

whereistejas · 2021-07-13T16:34:53Z

@ehuss Great! That seems to have taken care of that issue. Since, the windows CI also passed, can we assume that this works on Windows, too?

I think, now we can proceed with writing more tests, if needed. @alexcrichton also mentioned feature-gating and some extra documentation...

alexcrichton · 2021-07-13T18:16:38Z

src/cargo/core/manifest.rs

@@ -662,6 +667,7 @@ impl Target {

    pub fn bin_target(


I think this is something that we'll want to handle for at least the cdylib output as well, perhaps the staticlib output too. Overall I think that the filename handling from the TOML profile may need to happen at a lower layer than as a new method on constructors here (as otherwise it would need to be added as an argument to all constructors)

I would prefer to re-use the name field in the same struct, instead of creating a new field as I have done here. But, @ehuss, suggested not to do that as we don't know what kind of implications that might have.

I think what Alex is referring to here is that it could be generalized for all the target types, and that could be handled in some place like configure. I don't feel strongly about supporting that in the initial PR, though it would be good to keep that in mind.

alexcrichton · 2021-07-13T18:18:16Z

src/cargo/core/compiler/build_context/target_info.rs

        }
    }

    /// The filename for this FileType that Cargo should use when "uplifting"
    /// it to the destination directory.
    pub fn uplift_filename(&self, target: &Target) -> String {
+        // for binary crate type, `should_replace_hyphens` will always be false
        let name = if self.should_replace_hyphens {


I would agree that this is a bit subtle and I think it'd be best if this were handled a bit differently where there was an early-return or similar

alexcrichton · 2021-07-13T18:18:57Z

src/cargo/core/compiler/build_context/target_info.rs

-            target.name().to_string()
+            target
+                .get_binary_name()
+                .unwrap_or_else(|| target.name().to_string())
        };
        format!("{}{}{}", self.prefix, name, self.suffix)


This looks like it's automatically appending a prefix/suffix such as lib, .a, and .exe I think. If that's the case I'm not sure that filename is the right name for the key in the manifest since it's only influencing part of the filename, not the whole filename.

what about something like binary_name instead of filename? would that be more suitable?

We discussed some ideas for the name, but none seemed particularly good. file_stem is a strong option, but it may not work well with libraries. Another option is to allow name to support arbitrary values, but if it is a non-identifier, require a second field of crate-name.

We didn't feel strongly about any of the options we thought of, so I think leaving this as filename for now is fine, and we can maybe consider other options later.

src/cargo/core/compiler/mod.rs

whereistejas · 2021-07-14T10:26:47Z

Hi @ehuss / @alexcrichton , how do you want me to proceed with this, now?

alexcrichton · 2021-07-15T20:51:38Z

Reviewers like @ehuss and I have time to help out and provide some guidance in places where necessary, but I don't think either of us (or others) have the time to do 1 on 1 mentoring right now with detailed guidanced and detailed information about how to solve all concerns. Unfortunately that can amount to the reviewer doing most of the work of design and largely implementation, which can be very time consuming.

In some sense @whereistejas you'll need to take some liberties in taking feedback on this PR and incorporating the features and requests into the implementation yourself. If you have general questions about Cargo's or need assistance we can try to help, but I don't think either of us is in a position to provide the detailed level of advice you may be looking for on this.

whereistejas · 2021-07-17T14:15:49Z

Hi @alexcrichton, thank you for the candid response. Given, the small team managing this project, it is perfectly understandable, that you can dedicate only a limited amount of time, to helping out people with their PR. No hard feelings (I mean it). The point of doing open-source contributions is to help out maintainers such as yourself save time, not further eat into it. We are all on the same team here. :)

I will try to take this PR as close to the Finished state as I can, before asking for a review again. I'm almost finished, here, as I have already finished feature gating and adding the documentation. I just want to add a few more tests.

whereistejas · 2021-07-17T17:44:32Z

Hi @alexcrichton / @ehuss ,
This PR is ready from my side.

Tasks:

Add tests.
Feature gate and make unstable.
Update cargo-feature documentation.
Add appropriate comments where needed.

This is a short description of my implementation:

We are receiving the name the user expects to give the binary in a parameter called filename from cargo.toml.
This value is stored in the bin_name field of the TargetInner struct.
Instead of using bin_name to name the binaries themselves, we simply use it to create the hardlink in the target directory.
This is done using uplift_filename method.

The uplift_filename method is used in three places:
i. fn uplift_to - used for cargo build, this is our method of interest.
ii. fn clean - used for cargo clean. uplift_filename provides the correct name for the hardlink that needs to removed in this method.

cargo/src/cargo/ops/cargo_clean.rs

Lines 190 to 197 in 54bc19e

    
           // Remove the uplifted copy. 
        
           if let Some(uplift_dir) = uplift_dir { 
        
               let uplifted_path = uplift_dir.join(file_type.uplift_filename(target)); 
        
               rm_rf(&uplifted_path, config)?; 
        
               // Dep-info generated by Cargo itself. 
        
               let dep_info = uplifted_path.with_extension("d"); 
        
               rm_rf(&dep_info, config)?; 
        
           }

iii. fn bin_link_for_target - this method is only used when no Unit is found. We can safely assume that uplift_filename here will always return the crate name.

cargo/src/cargo/core/compiler/context/compilation_files.rs

Lines 304 to 307 in 54bc19e

    
           /// Returns the path to the executable binary for the given bin target. 
        
           /// 
        
           /// This should only to be used when a `Unit` is not available. 
        
           pub fn bin_link_for_target(

By simply renaming the hardlink, we don't disturb any other dependencies that other methods may have on the path field of the OutputFile object. Since, the user is only concerned with the final binary produced by cargo, using the hardlink, works out well.
The other binaries that are created in the deps/ folder, have the same name as the crate.

I have currently added three tests:

To check if we actually create the correct binary.
To check if the deps file has the correct content.
To check if cargo clean works.

I would like your input on deciding what the parameter, we use in Cargo.toml should be called. A few suggestions would be:

filename - This is what I am using, right now.
binary_name - This is quite suitable.
hardlink

ehuss

Can you add a few more tests?

Check that it is feature-gated. That is, try to run cargo build with a manifest where cargo-features is not set, and make sure it fails. Look for tests called gated() for some examples.
Check the JSON output for the artifact message in --message-format=json contains the correct path.
Check that fingerprinting works correctly. You can stuff this in one of the other tests, like simple_binary_name where after the first build, just run cargo build -v again and make sure the output says [FRESH] foo [..].
Check that cargo run is able to execute the binary.
Check that cargo test is able to test the binary. I think as written, it still uses the crate name in the filename?
Check that the CARGO_BIN_EXE_$name environment variable is set correctly for integration tests.
Check the CARGO_BIN_NAME environment variable. I'm uncertain what value this should have. I think based on the original use cases, this might want to be the new filename. I'm a little concerned about the increasing inconsistencies this is introducing though (like the --bin flag uses the bin name, not the filename).
Add some tests for cargo install and cargo uninstall. Make sure it installs the correct name and is able to uninstall it.

ehuss · 2021-07-23T18:34:12Z

src/cargo/core/compiler/build_context/target_info.rs

        }
    }

    /// The filename for this FileType that Cargo should use when "uplifting"
    /// it to the destination directory.
    pub fn uplift_filename(&self, target: &Target) -> String {
+        // for binary crate type, `should_replace_hyphens` will always be false
        let name = if self.should_replace_hyphens {


I think to make it clearer, this could check if get_binary_name returns a value, and if it does, use that, then check should_replace_hyphens, and then the final else would return target.name(). That should make it more explicit.

ehuss · 2021-07-23T18:38:31Z

src/cargo/core/compiler/build_context/target_info.rs

-            target.name().to_string()
+            target
+                .get_binary_name()
+                .unwrap_or_else(|| target.name().to_string())
        };
        format!("{}{}{}", self.prefix, name, self.suffix)


We discussed some ideas for the name, but none seemed particularly good. file_stem is a strong option, but it may not work well with libraries. Another option is to allow name to support arbitrary values, but if it is a non-identifier, require a second field of crate-name.

We didn't feel strongly about any of the options we thought of, so I think leaving this as filename for now is fine, and we can maybe consider other options later.

ehuss · 2021-07-23T18:41:45Z

src/cargo/core/compiler/context/compilation_files.rs

+                .unstable_features()
+                .require(Feature::different_binary_name())?;


This is a bit of an unusual place to put the unstable check, as this is pretty low-level. Usually the unstable checks are placed towards the front end if possible, such as in toml/mod.rs or toml/targets.rs or Manifest::feature_gate. I think toml/targets.rs would probably be a good place.

ehuss · 2021-07-23T18:57:20Z

src/cargo/core/manifest.rs

@@ -662,6 +667,7 @@ impl Target {

    pub fn bin_target(


I think what Alex is referring to here is that it could be generalized for all the target types, and that could be handled in some place like configure. I don't feel strongly about supporting that in the initial PR, though it would be good to keep that in mind.

src/doc/src/reference/unstable.md

ehuss · 2021-07-23T19:10:30Z

tests/testsuite/binary_name.rs

+#[cargo_test]
+fn check_deps_content() {


Can this test be merged with simple_binary_name? Each test has a small cost, and we don't want to add them too extravagantly.

So, basically, I should create new tests only if the content of cargo.toml changes. Is that correct? We want to perform as many checks as we can in the same test method (within good reason) as long as all of them can use the same cargo.toml content.

Hm, I thought I responded to this, but I think my comment got eaten. There aren't any specific rules. But yea, when different steps of a test have the same Cargo.toml, I usually place them together in the same test. There is a balance between having too many individual tests (which slows things down), and having too many steps in a single test (which can make the test hard to work on and debug). Different people will have different opinions about exactly how to structure things, and it is mostly just a judgement call.

whereistejas · 2021-07-24T07:51:51Z

whereistejas · 2021-07-25T10:43:51Z

Hi @ehuss,

Check that the CARGO_BIN_EXE_$name environment variable is set correctly for integration tests.

Writing the test and fix checking CARGO_BIN_EXE_<name> is indeed, tricky. I think, it would be best if let it be as it is. We can mention this inconsistency in the documentation.

From what I can tell, it is kind of tricky to assign the value we get from filename parameter to the env <name> variable. This is happening in the following piece of code:

diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs
index 1687a5d2a..6501231c9 100644
--- a/src/cargo/core/compiler/mod.rs
+++ b/src/cargo/core/compiler/mod.rs
@@ -980,7 +980,11 @@ fn build_base_args(
             let exe_path = cx
                 .files()
                 .bin_link_for_target(bin_target, unit.kind, cx.bcx)?;
-            let key = format!("CARGO_BIN_EXE_{}", bin_target.name());
+            let name = unit
+                .target
+                .get_binary_name()
+                .unwrap_or(bin_target.name().to_string());
+            let key = format!("CARGO_BIN_EXE_{}", name);
             cmd.env(&key, exe_path);
         }
     }

This code is called once for each unit as we are looping over each root present in the build context (bcx). Our source code from the src/ directory forms the first unit and integretion tests from tests/ directory form the second unit. The filename parameter is not available to us in the second unit, as the target_type of this unit is test and not bin. The filename parameter is only available for bin target type. I think, we are moving into really slippy territory here.

Would it be problematic if we had $CARGO_BIN_EXE_foo when the actual executable name is 007bar?

One possible alternative, is to use add the filename parameter to the [[test]] section, also. We can throw an error if the filename parameters from both the sections [[bin]] and [[test]] don't match. This also gives us the opportunity to cater @alexcrichton's suggestion of generalizing the filename parameter to all types of targets.

julihoh · 2021-07-29T11:51:10Z

Just chiming in to say that supporting cdylibs (as suggested in the original issue and by @alexcrichton here) would be awesome. Just for your consideration!

Cheers

ehuss · 2021-07-30T20:34:35Z

src/doc/src/reference/unstable.md

+restrictions placed on crate names. For example, the crate name must use only `alphanumeric` characters
+or `-` or `_`, and cannot be empty.
+
+The `filname` parameter should **not** include the binary extension, `cargo` will figure out the appropriate


Suggested change

The `filname` parameter should **not** include the binary extension, `cargo` will figure out the appropriate

The `filename` parameter should **not** include the binary extension, `cargo` will figure out the appropriate

ehuss · 2021-07-30T20:51:44Z

Sorry, I'm a bit confused about the questions for CARGO_BIN_EXE. Wouldn't the key name be something like bin_target.get_binary_name().unwrap_or(bin_target.name().to_string());? I'm not sure why it is using unit.target, as that is the wrong target.

Also, looking at the usage of get_binary_name, it looks like it is always immediately converts None to target.name(), perhaps it shouldn't return an Option? I may also suggest remove the get_ prefix, as that is not the recommended style in Rust (and isn't used in any other methods of that struct) (see C-GETTER).

whereistejas · 2021-07-31T14:13:35Z

Sorry, I'm a bit confused about the questions for CARGO_BIN_EXE. Wouldn't the key name be something like bin_target.get_binary_name().unwrap_or(bin_target.name().to_string());? I'm not sure why it is using unit.target, as that is the wrong target.

@ehuss this is right! Thanks for this, it helped me out a bunch. I have covered all the tests, now. They are all passing. I had some trouble figuring out how to handle windows vs. unix paths, thankfully I saw the #[cfg(windows)] option in one of the other tests.

The only test I'm a little sceptical about is this: check_msg_format_json. Please let me know if there is any other pending stuff.

src/doc/src/reference/unstable.md

tests/testsuite/binary_name.rs

bors · 2021-08-06T21:07:58Z

☔ The latest upstream changes (presumably #9118) made this pull request unmergeable. Please resolve the merge conflicts.

whereistejas · 2021-08-07T07:13:40Z

Hi @ehuss, I have fixed all the issues you had mentioned in your comments.

ehuss

Thanks, just a few small updates and I think this should be good to go. Can you also squash all the commits?

tests/testsuite/binary_name.rs

src/doc/src/reference/unstable.md

whereistejas · 2021-08-10T19:49:10Z

Okay, so I tried to git rebase and I completely messed this up.

I recovered from this mishap, thank God.

whereistejas · 2021-08-10T20:40:25Z

Hi @ehuss, is it possible to do a squash merge from your side? 2d6f537 is a merge commit, I cannot squash it using rebase.

…e crate name

ehuss · 2021-08-11T21:11:34Z

No worries. Git can be finicky sometimes. Thanks! 🎉

@bors r+

bors · 2021-08-11T21:11:35Z

📌 Commit 9e2790f has been approved by ehuss

bors · 2021-08-11T21:11:48Z

⌛ Testing commit 9e2790f with merge 3d3ddd2...

bors · 2021-08-11T21:45:04Z

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 3d3ddd2 to master...

whereistejas · 2021-08-11T21:52:47Z

Thanks @ehuss for all of your help! I really appreciate the patience from your side. I just have one last question, what will we have to do make this feature a stable feature?

ehuss · 2021-08-11T22:40:05Z

There is not a well established protocol for stabilizing. Generally, features land on nightly for a while, and people should test it out and report their experiences. Preferably we would like to see that a feature has at least some degree of broad interest, and isn't being added for just one project. The unresolved issues listed in #9778 should either get resolved or have some good explanation of why they don't need to be resolved.

Update cargo 8 commits in b51439fd8b505d4800a257acfecf3c69f81e35cf..e96bdb0c3d0a418e7fcd7fbd69be08abf830b4bc 2021-08-09 18:40:05 +0000 to 2021-08-17 22:58:47 +0000 - Support using rustbot to ping the Windows group (rust-lang/cargo#9802) - Show information about abnormal `fix` errors. (rust-lang/cargo#9799) - Bump jobserver. (rust-lang/cargo#9798) - Render build-std web links as hyperlinks (rust-lang/cargo#9795) - Teach cargo to failfast on recursive/corecursive aliases (rust-lang/cargo#9791) - Fix value-after-table error with profiles. (rust-lang/cargo#9789) - Fix plugin registrar change. (rust-lang/cargo#9790) - Ability to specify the output name for a bin target different from the crate name (rust-lang/cargo#9627)

rust-highfive assigned Eh2406 Jun 26, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 26, 2021

whereistejas commented Jul 11, 2021

View reviewed changes

ehuss assigned ehuss and unassigned Eh2406 Jul 13, 2021

ehuss reviewed Jul 13, 2021

View reviewed changes

alexcrichton reviewed Jul 13, 2021

View reviewed changes

whereistejas force-pushed the many-bin-names branch from 8e95324 to 65194e6 Compare July 13, 2021 18:47

whereistejas requested review from ehuss and alexcrichton July 15, 2021 17:53

ehuss reviewed Jul 23, 2021

View reviewed changes

ehuss reviewed Jul 30, 2021

View reviewed changes

whereistejas requested a review from ehuss August 5, 2021 04:32

ehuss reviewed Aug 6, 2021

View reviewed changes

src/doc/src/reference/unstable.md Outdated Show resolved Hide resolved

tests/testsuite/binary_name.rs Outdated Show resolved Hide resolved

tests/testsuite/binary_name.rs Outdated Show resolved Hide resolved

whereistejas force-pushed the many-bin-names branch from 882a1d2 to 4f9737b Compare August 6, 2021 20:02

ehuss mentioned this pull request Aug 10, 2021

Tracking Issue for different-binary-name #9778

Open

3 tasks

ehuss reviewed Aug 10, 2021

View reviewed changes

tests/testsuite/binary_name.rs Outdated Show resolved Hide resolved

tests/testsuite/binary_name.rs Outdated Show resolved Hide resolved

src/doc/src/reference/unstable.md Outdated Show resolved Hide resolved

whereistejas force-pushed the many-bin-names branch 3 times, most recently from 0fbc30d to 55de0a0 Compare August 10, 2021 19:43

whereistejas force-pushed the many-bin-names branch 2 times, most recently from 1a93142 to 2d92a9b Compare August 10, 2021 20:32

Ability to specify the output name for a bin target different from th…

9e2790f

…e crate name

ehuss force-pushed the many-bin-names branch from 2d6f537 to 9e2790f Compare August 11, 2021 21:10

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 11, 2021

bors merged commit 3d3ddd2 into rust-lang:master Aug 11, 2021

ehuss mentioned this pull request Aug 11, 2021

Ability to specify the output name for a bin target different from the crate name #1706

Closed

ehuss mentioned this pull request Aug 18, 2021

Update cargo rust-lang/rust#88127

Merged

ehuss added this to the 1.56.0 milestone Feb 6, 2022

		.unstable_features()
		.require(Feature::different_binary_name())?;

	The `filname` parameter should not include the binary extension, `cargo` will figure out the appropriate
	The `filename` parameter should not include the binary extension, `cargo` will figure out the appropriate

		#[cargo_test]
		fn check_deps_content() {

Ability to specify the output name for a bin target different from the crate name #9627

Ability to specify the output name for a bin target different from the crate name #9627

Conversation

whereistejas commented Jun 26, 2021 • edited Loading

rust-highfive commented Jun 26, 2021

alexcrichton commented Jun 29, 2021

whereistejas commented Jul 3, 2021 • edited Loading

whereistejas commented Jul 8, 2021

Eh2406 commented Jul 8, 2021

ehuss commented Jul 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whereistejas commented Jul 11, 2021

whereistejas commented Jul 12, 2021

Choose a reason for hiding this comment

whereistejas commented Jul 13, 2021

whereistejas commented Jul 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whereistejas commented Jul 14, 2021

alexcrichton commented Jul 15, 2021

whereistejas commented Jul 17, 2021 • edited Loading

whereistejas commented Jul 17, 2021 • edited Loading

ehuss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whereistejas commented Jul 24, 2021 • edited Loading

whereistejas commented Jul 25, 2021 • edited Loading

julihoh commented Jul 29, 2021

Choose a reason for hiding this comment

ehuss commented Jul 30, 2021

whereistejas commented Jul 31, 2021

bors commented Aug 6, 2021

whereistejas commented Aug 7, 2021

ehuss left a comment

Choose a reason for hiding this comment

whereistejas commented Aug 10, 2021 • edited Loading

whereistejas commented Aug 10, 2021 • edited Loading

ehuss commented Aug 11, 2021

bors commented Aug 11, 2021

bors commented Aug 11, 2021

bors commented Aug 11, 2021

whereistejas commented Aug 11, 2021 • edited Loading

ehuss commented Aug 11, 2021

whereistejas commented Jun 26, 2021 •

edited

Loading

whereistejas commented Jul 3, 2021 •

edited

Loading

ehuss commented Jul 8, 2021 •

edited

Loading

whereistejas commented Jul 13, 2021 •

edited

Loading

whereistejas commented Jul 17, 2021 •

edited

Loading

whereistejas commented Jul 17, 2021 •

edited

Loading

whereistejas commented Jul 24, 2021 •

edited

Loading

whereistejas commented Jul 25, 2021 •

edited

Loading

whereistejas commented Aug 10, 2021 •

edited

Loading

whereistejas commented Aug 10, 2021 •

edited

Loading

whereistejas commented Aug 11, 2021 •

edited

Loading