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

usage of javascript/node to allow cross-platform building #5825

Open
wants to merge 6 commits into
base: x
Choose a base branch
from

Conversation

Osiris-Team
Copy link

@Osiris-Team Osiris-Team commented Sep 13, 2024

At the current state the build fails when following the simple instructions in the README on Windows.
This fixes it.

Copy link

codesandbox bot commented Sep 13, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link

what-the-diff bot commented Sep 13, 2024

PR总结

  • 更新了README.md
    • 添加了Python安装的要求
  • 引入了clean_workspace.py脚本
    • 用于清理工作区, 替换先前使用的Shell脚本
  • 删除了旧的Shell脚本 clean_workspace.sh
  • 删除了旧的Shell脚本copy-injected.sh并引入copy_injected.py
    • 用来复制注入的文件
  • 添加了postinstall.py脚本
    • 用于处理安装后步骤, 替换先前的postinstall.sh
  • 更新了package.json
    • 使用Python脚本替代Shell脚本进行清理和复制任务, 确保跨平台兼容性
  • 添加了cross-env-shell依赖
    • 以跨平台方式管理环境变量

@sidmorizon
Copy link
Contributor

Thanks for your contribution @Osiris-Team

But we're not planning to introduce Python scripts into the project, you can consider converting the Python scripts to Node scripts.

@Osiris-Team
Copy link
Author

Osiris-Team commented Sep 14, 2024

Oh yeah that makes sense, didn't think about that!
-> Fixed

@huhuanming
Copy link
Contributor

huhuanming commented Sep 14, 2024

Thanks for your contribution @Osiris-Team.

but development/scripts/copy_injected.js is still a python file.

Maybe you can make modifications based on this draft PR #5830

@Osiris-Team
Copy link
Author

my bad forgot to copy over, fixed

@@ -11,9 +11,10 @@
"node": ">=20"
},
"scripts": {
"setup:env": "bash -c 'if [ ! -f .env ]; then cp .env.example .env; fi'",
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to write it like this?

"setup:env": "node -e \"const fs=require('fs');if(!fs.existsSync('.env'))fs.copyFileSync('.env.example','.env');\"",

@Osiris-Team Osiris-Team changed the title usage of python to allow cross-platform building usage of javascript/node to allow cross-platform building Sep 14, 2024
@huhuanming
Copy link
Contributor

and apps/desktop/dist/build_keytar.sh need to be fixed.

After all the files have been modified, I will help you package and test it.

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.

4 participants