The first approach where you access legacyPackages is actually more efficient and generally preferable, since you will be reusing the already existing instance of nixpkgs.
The second approach is less efficient because you are creating a new nixpkgs instance, meaning that because of the import statement, nix will re-evaluate the code in nixpkgs’ default.nix and keep a separate instance in memory, in addition to the one that was already passed to your outputs function and that’s also used by home-manager (because of how you configured the inputs, which is generally what you’d want).
The fact that the package set is called legacy, is not because its use is deprecated or such, it’s because the packages output in a flake is only supposed to contain derivations, but nixpkgs’ top level expression also contains other things (like functions and attrsets containing derivations), and so if we’d put all of that in packages, nix flake check would complain. And so a separate output was created that doesn’t have this requirement.
That is only a problem when the dependency flake already uses legacyPackages. home-manager.lib.homeManagerConfiguration requires passing a Nixpkgs instance explicitly so it does not suffer from the issue.
You may encounter it when you use home-manager’s package outputs but that does not appear to be the case here.
In my opinion, dependency injection is a good idea and outputs like packages that do not support it are ugly and should only ever be used for ad-hoc commands nix run. Pulling packages into system configurations is more cleanly achieved with overlays (pure ones, not just superimposition of packages). The downside is that dependency then loses the ability to pin Nixpkgs.
Also since legacyPackages is already instantiated, it does not allow you to parametrize the Nixpkgs instance, e.g. setting config.
Well, Nix is a lazy language so if you do not use legacyPackages anywhere else in the evaluation, you will still only have a single instance.