Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jujutsu: support darwin guidelines for config placement #5416

Merged

Conversation

mrnossiom
Copy link
Contributor

@mrnossiom mrnossiom commented May 19, 2024

Follow up to #5207, fixing jujutsu module on darwin targets.

Closes #5544 (duplicate)

Description

If the module is evaluated on a Darwin platform, place config in Library/Application Support instead of unsupported XDG directory.

If someone could test it on Darwin, it would be great (e.g. @bnjmnt4n) :

nix develop --ignore-environment .#jujutsu-example-config
nix develop --ignore-environment .#jujutsu-empty-config

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

Maintainer CC

@shikanime

@mrnossiom
Copy link
Contributor Author

Could someone hint me on what's wrong with the test ?

@mrnossiom mrnossiom closed this May 19, 2024
@mrnossiom mrnossiom reopened this May 19, 2024
@bnjmnt4n
Copy link

LGTM, but I'm not sure about the tests either...

@huwaireb
Copy link

huwaireb commented Jun 18, 2024

For the tests:

I'm getting Expected home-files/Library/Application to exist but it was not found. when running nix develop --ignore-environment .#all locally as the failure reason for Jujutsu example-config.nix test.

@mrnossiom
Copy link
Contributor Author

mrnossiom commented Jun 18, 2024

I just forgot to escape the space in Application Support. It's spliced into two arguments. Commit on the way.

@mrnossiom mrnossiom force-pushed the fix/jujustu-support-darwin-config-dirs branch 2 times, most recently from 49676ec to 9663ce3 Compare June 18, 2024 17:21
@mrnossiom
Copy link
Contributor Author

@huwaireb Could you please rerun the tests with the latest commit? I don't know why, the CI gives cryptic errors.

@huwaireb
Copy link

huwaireb commented Jun 18, 2024

@huwaireb Could you please rerun the tests with the latest commit? I don't know why, the CI gives cryptic errors.

erm, wut?

jujutsu-empty-config: OK
jujutsu-example-config: OK

Screenshot 2024-06-19 at 01 29 41

The CI seems to be giving out unrelated errors?

External messages

rmu — Today at 1:37 AM
not sure if this is a stupid idea, but would rebasing perhaps fix it? I'm assuming it's running the CI in your branch not > applying your PR as a patch and then running the CI
idk how github ci does it tbh
the errors seem totally unrelated though, it would make sense if something changed for MacOS specifically since May.
ok so github allows you to see the workflow file that was used, yes it seems to be outdated. See:
https://github.com/nix-community/home-manager/actions/runs/9569434817/workflow?pr=5416
vs
https://github.com/nix-community/home-manager/blob/master/.github/workflows/test.yml

@mrnossiom mrnossiom force-pushed the fix/jujustu-support-darwin-config-dirs branch 2 times, most recently from 373828d to 7531921 Compare June 18, 2024 22:03
@huwaireb
Copy link

huwaireb commented Jun 18, 2024

MacOS CI seems to be broken for the past ~3-4 days? Due to swift-5.8.

Found a mention to what could be the culprit:
NixOS/nixpkgs#316075 (comment).

requires a nixpkgs fix.

opened an home-manager issue, see #5550.

@cdmistman
Copy link

the above has been fixed, and it should be available in the latest locked nixpkgs in master. i think if @mrnossiom merges/rebases master then the swift build failure shouldn't cause ci to fail

@mrnossiom mrnossiom force-pushed the fix/jujustu-support-darwin-config-dirs branch from 7531921 to 1deda06 Compare July 24, 2024 16:51
Follow up to nix-community#5207, fixing jujutsu module on darwin targets.
@rycee rycee force-pushed the fix/jujustu-support-darwin-config-dirs branch from 1deda06 to a163795 Compare July 28, 2024 20:52
@rycee rycee merged commit ea72cf5 into nix-community:master Jul 28, 2024
2 of 3 checks passed
@rycee
Copy link
Member

rycee commented Jul 28, 2024

Thanks! Squashed and merged to master now 🙂

@mrnossiom mrnossiom deleted the fix/jujustu-support-darwin-config-dirs branch July 30, 2024 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants