Inconsistent tailing slash in remapping name and path

[Troublor]

Hi, I notice that when formatting remappings in the compiler settings, a slash / is pushed if the remapped path does not end with /.

compilers/src/remappings.rs

Lines 160 to 162 in 6528e4a

	if !s.ends_with('/') {
	s.push('/');
	}

This may lead to incorrect path remapping and cause compiler failure.
For example, if the original remapping is @0x/contracts-utils=/home/cluracan/code/0x-monorepo/node_modules/@0x/contracts-utils, and one more / is added at the end.
The remapping now becomes: @0x/contracts-utils=/home/cluracan/code/0x-monorepo/node_modules/@0x/contracts-utils/.

If the contract has one import like this: import "@0x/contracts-utils/contracts/src/v06/LibBytesV06.sol";
The imported path after remapping is /home/cluracan/code/0x-monorepo/node_modules/@0x/contracts-utils//contracts/src/v06/LibBytesV06.sol.
Note that there is one redundant / in the remapped path and this leads to File not found errors in the compiler.

I encounter this issue when trying to compile mainnet contract 0xDef1C0ded9bec7F1a1670819833240f027b25EfF with the source code provided by Etherscan API.

[mattsse]

could you please prepare a simple repro for this?

[Troublor]

@mattsse Please check this repo for reproduce this issue:
https://github.com/Troublor/foundry-rs-compilers-issue47

I put some comments in the src/main.rs to illustrate the situation.

[mattsse]

interesting, looks like this affects solc versions < 0.8.8

I think we should remove the trailing / then, in serialization.
looks like pre and post missing trailing / is handled properly, so there's probably some join bug in solc that has been fixed via the base path/allow path features in 0.8.8

This makes me think that we actually want the inverse here? if it ends with a /, remove that

could you convert this issue into a test and open a PR so we can fix this?

[Troublor]

Sure I can have a PR, but
just be sure, what is the reason for appending a trailing / in the first place? I feel there must be a reason (e.g., handle some corner cases) in the previous commit.

[BlazeWasHere]

adding to this issue, there are few more places where a trailing / is added (not sure if its removed later in the call frame)

compilers/src/remappings.rs

Lines 178 to 181 in 342f713

	/// However, there are additional rules/assumptions when it comes to determining if a candidate
	/// should in fact be a remapping:
	///
	/// All names and paths end with a trailing "/"

compilers/src/remappings.rs

Lines 245 to 263 in 342f713

	for candidate in candidates {
	if let Some(name) = candidate.window_start.file_name().and_then(|s| s.to_str()) {
	insert_prioritized(
	&mut all_remappings,
	format!("{name}/"),
	candidate.source_dir,
	);
	}
	}
	}
	all_remappings
	.into_iter()
	.map(|(name, path)| Remapping {
	context: None,
	name,
	path: format!("{}/", path.display()),
	})
	.collect()

compilers/src/remappings.rs

Lines 863 to 866 in 342f713

	// helper function for converting path bufs to remapping strings
	fn to_str(p: std::path::PathBuf) -> String {
	format!("{}/", p.display())
	}