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

project-lemonlime: init at 0.3.5 #329904

Closed

Conversation

Sigmanificient
Copy link
Member

@Sigmanificient Sigmanificient commented Jul 25, 2024

Description of changes

Fixes: #329546

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Jul 25, 2024
@Bot-wxt1221
Copy link
Member

Bot-wxt1221 commented Jul 26, 2024

@Sigmanificient We should provide a way to user that make sure the compiler's path won't change. It won't detect PATH every time when started but just save them into a config file. It is in ~/.config/LemonLime/lemon.conf

For me, a warp shell is the best.

It could be better if we can generate config file by nix. It should be included in home-manager.

Anyway, please support darwin as well.

@Sigmanificient
Copy link
Member Author

Well we cant really control anything outside the package, but the compiler path would not change unless the user changes its version

@Bot-wxt1221
Copy link
Member

@Sigmanificient I means that we could create a wrap shell and provide a option in nixpkgs for user to choose their compiler. Or it will be a mess if the compiler path is changed.

@Sigmanificient
Copy link
Member Author

Sigmanificient commented Jul 26, 2024

I dont understand what you are referring by wrap shell, but setting compiler system-wide is also problematic

@Bot-wxt1221
Copy link
Member

Bot-wxt1221 commented Jul 26, 2024

@Sigmanificient I means we provide the compiler path to lemonlime in some ways that user won't worry about the diffrent compiler path.

We use bubblewrap or we just patch lemonlime and allow it reading compiler path from environment value.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/4434

@Bot-wxt1221
Copy link
Member

Darwin need to load more library. However I haven't got device to test. Could you mark it as
broken?

@Sigmanificient

@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by the maintainer of the package it changes label Aug 20, 2024
Copy link
Contributor

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your fixes isn't written in a way that GitHub recognizes it. It should be written:

Fixes #329546

The filename, like the pname, should be hyphenated instead of underscored.

pkgs/by-name/pr/project_lemonlime/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/pr/project_lemonlime/package.nix Outdated Show resolved Hide resolved
@Sigmanificient Sigmanificient changed the title project_lemonlime: init at 0.3.5 project-lemonlime: init at 0.3.5 Aug 21, 2024
Copy link
Contributor

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I don't think a meta.homepage is required?

@Pandapip1 Pandapip1 added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Aug 21, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Aug 21, 2024
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1942


nativeBuildInputs = [
cmake
qt5.wrapQtAppsHook
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qt 6 should be preferred if there are no issues with it. Upstream provided https://github.com/Project-LemonLime/Project_LemonLime/blob/master/CMakeLists.txt#L47

(Also qt6.qtwayland!)

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Sep 7, 2024
@Bot-wxt1221
Copy link
Member

#345422

@Bot-wxt1221 Bot-wxt1221 closed this Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package request: LemonLime
7 participants