[Bug][RFC] Confusing naming and implementation inconsistencies with the `hostname` field

[cyqsimon]

I was looking at my own bash import code, and saw that I was not populating the hostname field.

https://github.com/ellie/atuin/blob/f2a496848afa358ec985a74392f6a2b1403982f8/atuin-client/src/import/bash.rs#L73-L82

So I thought, huh, wouldn't it make sense to just use the hostname of the machine where importing is happening? This took me down a long rabbit hole which eventually brought me here.

I soon realised that this is already done further down the call stack:

https://github.com/ellie/atuin/blob/f2a496848afa358ec985a74392f6a2b1403982f8/atuin-client/src/history.rs#L49-L53

But I also saw that the hostname field isn't really what we would canonically call the "hostname"; rather it's a hostname:username combo. This immediately raises red flags in my head, and I thought "surely someone is going to fuck up here". And it seems like someone already has:

https://github.com/ellie/atuin/blob/f2a496848afa358ec985a74392f6a2b1403982f8/atuin-client/src/import/resh.rs#L133

Now I'm not familiar with Resh at all, but looking at the ReshEntry struct, there does seem to be a login field in addition to the host field:

https://github.com/ellie/atuin/blob/f2a496848afa358ec985a74392f6a2b1403982f8/atuin-client/src/import/resh.rs#L16-L34

... which means the current Resh import code is incorrect no? Or am I going crazy?

Similarly, it appears that the import code for nu_histdb and zsh_histdb have the same problem, unless those database files also chose the same confusing naming for their fields:

https://github.com/ellie/atuin/blob/f2a496848afa358ec985a74392f6a2b1403982f8/atuin-client/src/import/nu_histdb.rs#L57-L69

https://github.com/ellie/atuin/blob/f2a496848afa358ec985a74392f6a2b1403982f8/atuin-client/src/import/zsh_histdb.rs#L99-L105

---

Anyways, I thought I'd report this as an issue, because naming something it's not is always a disaster waiting to happen. A patchy fix is probably not too difficult, but that's less than ideal IMO.

Ideally the field name in the table should be renamed something else, or maybe the hostname and the username should just be stored separately. This is the cleanest and prevents future errors. However this comes with the issues of compatibility and migration, so I'd like to discuss this before making any big changes.

[ellie]

The original intent for the hostname field was for it to be unique per host - it started off as just the hostname, but ended up including the username as an earlier revision of sync was much fussier

We will be changing it at some point, but atm it isn't really used for much other than as part of sync - and all that requires is a unique hash per host. So it not being used entirely consistently isn't a big problem + is one we can easily fix

For your own code, feel free to use the hostname of the machine importing, or any other string - it doesn't really matter for imports

[cyqsimon]

We will be changing it at some point, but atm it isn't really used for much other than as part of sync - and all that requires is a unique hash per host. So it not being used entirely consistently isn't a big problem + is one we can easily fix

Isn't it also used for host filtering? Or am I missing something here?

https://github.com/ellie/atuin/blob/f2a496848afa358ec985a74392f6a2b1403982f8/atuin-client/src/database.rs#L249-L254

Wouldn't an inconsistent hostname field format cause any history imported using these import methods to be not listed on the same machine when the host filter mode is applied?

[ellie]

Ahh shit yeah, you're right - I forgot about that 🙃

Either way, going forwards it should be fine to just store hostnames + adjust the filter query for the older data