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

stm32cubemx: wrap with FHSEnv #280833

Merged
merged 2 commits into from
Jan 18, 2024
Merged

Conversation

asmir-abdulahovic
Copy link
Contributor

Description of changes

As of recently STM32CubeMX is using JxBrowser for, now mandatory, user authentication.
Same browser is extracted from STM32CubeMX install binaries into ~/.stm32cubemx directory and depends on the system shared libraries.

This PR wrapps STM32CubeMX with FHSEnv in order call JxBrowser.
Package remains mostly the same except a minor change where stdenv is replaced by stdenvNoCC.

STM32CubeMX login page takes a while to appear on Wayland (Sway) but works mostly fine.

Fixes: #272342

cc @angaz @wucke13

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.05 Release Notes (or backporting 23.05 and 23.11 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.

Copy link
Member

@angaz angaz left a comment

Choose a reason for hiding this comment

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

Tested running the application using nixpkgs-review.
Tested creating a new project, and generating the code. The login window opened, it didn't take in my key presses very well. I had to type the same key multiple times before it accepted a key. Very annoying for the password, but I would consider this a bug in the app, not the nix configuration.

From what I can tell, it's working as expected, and the mentioned bug is resolved.

@asmir-abdulahovic
Copy link
Contributor Author

asmir-abdulahovic commented Jan 14, 2024

Tested running the application using nixpkgs-review. Tested creating a new project, and generating the code. The login window opened, it didn't take in my key presses very well. I had to type the same key multiple times before it accepted a key. Very annoying for the password, but I would consider this a bug in the app, not the nix configuration.

From what I can tell, it's working as expected, and the mentioned bug is resolved.

I found copy/paste to be best solution for credentials input.
I have a feeling those issues are related to GPU acceleration in JxBrowser i.e. Chromium.
It's possible to disable hwaccel by using chrome cmdline flags, but exec string is hardcoded in STM32CubeMX and since login is required only once I think it's not worth to pursue further.

Thanks for testing and review.

Copy link
Member

@angaz angaz left a comment

Choose a reason for hiding this comment

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

Cool. Thanks for updating the platforms.
Everything looks good to me.

I'm not certain if the commit history is correct. Should changing the platforms be in it's own commit?

@asmir-abdulahovic
Copy link
Contributor Author

asmir-abdulahovic commented Jan 15, 2024

Cool. Thanks for updating the platforms. Everything looks good to me.

I'm not certain if the commit history is correct. Should changing the platforms be in it's own commit?

I squashed it, but I guess it would make more sense to have separate commit now that I think more about it.
Thanks for approving it again, so I'll not touch it anymore.

@angaz
Copy link
Member

angaz commented Jan 15, 2024

I squashed it, but I guess it would make more sense to have separate commit now that I think more about it. Thanks for approving it again, so I'll not touch it anymore.

Yeah, my comment was more for future reviewers. In case they don't see that it's been changed. Maybe it's something they would want.

@asmir-abdulahovic
Copy link
Contributor Author

I squashed it, but I guess it would make more sense to have separate commit now that I think more about it. Thanks for approving it again, so I'll not touch it anymore.

Yeah, my comment was more for future reviewers. In case they don't see that it's been changed. Maybe it's something they would want.

Understand, added separate commit.

Copy link
Member

@angaz angaz left a comment

Choose a reason for hiding this comment

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

Cool. Thanks

@ofborg ofborg bot requested a review from angaz January 16, 2024 02:12
@ajs124 ajs124 merged commit e4a70f5 into NixOS:master Jan 18, 2024
22 checks passed
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.

stm32cubemx: Can't initiate JxBrowser engine
6 participants