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

#693: Setup file not working on Mac #693

Merged
merged 7 commits into from
Oct 28, 2024
Merged

Conversation

blizzarac
Copy link
Contributor

@blizzarac blizzarac commented Oct 15, 2024

Tested on mac

  1. ide alias is not working like this -> Changing alias from source to ide to directly to ide
  2. IDE_URL was missing -> Added for current folder

@CLAassistant
Copy link

CLAassistant commented Oct 15, 2024

CLA assistant check
All committers have signed the CLA.

@blizzarac blizzarac changed the title Alias is pointing to source command instead of ide application #693: Alias is pointing to source command instead of ide application Oct 15, 2024
@blizzarac blizzarac changed the title #693: Alias is pointing to source command instead of ide application #693: Setup file not working on Mac Oct 15, 2024
@jan-vcapgemini jan-vcapgemini added macOS specific for Apple MacOS bugfix labels Oct 15, 2024
@jan-vcapgemini jan-vcapgemini self-assigned this Oct 15, 2024
@coveralls
Copy link
Collaborator

coveralls commented Oct 16, 2024

Pull Request Test Coverage Report for Build 11539868974

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 66.774%

Totals Coverage Status
Change from base Build 11527033206: 0.02%
Covered Lines: 6267
Relevant Lines: 9038

💛 - Coveralls

@hohwille
Copy link
Member

hohwille commented Oct 18, 2024

@blizzarac thanks for testing IDEasy (in beta phase already) and trying to help by creating a PR 👍
Could you please first describe the error you were experiencing on MacOS?
It is definitely wrong to remove the source statement from the script.
I can explain that in detail why we need to source the script (see also #63) but first I would like to understand what was going wrong.

IDE_URL was missing -> Added for current folder

I guess you mean IDE_ROOT.
That is definitely a known issue for Linux and Mac you we were already thinking about fixing this in the way you did.
However, currently we have a OS specific solution for that on Windows setting this in the Windows System Environment Variables after calling the setup Bash script (see https://github.com/devonfw/IDEasy/blob/main/cli/src/main/package/setup.bat). Therefore we should add an OS check and avoid doing that on Windows.

if [ "${OSTYPE}" = "cygwin" ] || [ "${OSTYPE}" = "msys" ]; then
...
else
...
fi

Do you want to do that and update your PR? Otherwise, we are happy to take over your work and create a new PR fixing that and hopefully include that into the planned 2024.10.001-beta release.

@hohwille
Copy link
Member

See also #423

@hohwille
Copy link
Member

@blizzarac as you seem to be quite busy with other stuff, I would suggest the following:

  • we create another PR for adding IDE_ROOT during setup for Linux and MacOS.
  • you only need to provide a small explanation of the error that you were facing that lead you to removing the source instruction. If there is some other "bug" than the missing IDE_ROOT, we can fix that. A comment would even be sufficient if you have no time. Otherwise creating a new bug would be even better.

@blizzarac
Copy link
Contributor Author

@hohwille sorry about, was a busy week. I think the problems that I had, had to do with my chaotic setup before I talked to @jan-vcapgemini and found out that I had to put everything in a _ide folder. Maybe this is the bug here (this was missing in the documentation)

* 'main' of github.com:blizzarac/IDEasy:
  devonfw#533: Add autocompletion of exit in ide shell (devonfw#707)
  Update advanced-tooling-windows.adoc
  simplify FlagProperty usage and allow ToolCommandlets to have long options (devonfw#644)
  Update CHANGELOG.adoc: prepare 2024.10.001
  devonfw#685: update dependencies, added missing deps to LICENSE.adoc, doc rework, jakarta migration (devonfw#686)
  devonfw#689: Added urls to tests (devonfw#695)
* 'alias_bug' of github.com:blizzarac/IDEasy:
@blizzarac
Copy link
Contributor Author

I have pushed a new version removing the source parts and added the IDE_ROOT with OS guards, hope that works for you

Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

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

@blizzarac thank you so much for the update of your PR. This is now perfect and ready for merge 👍

@hohwille hohwille added this to the release:2024.10.001 milestone Oct 28, 2024
@hohwille hohwille merged commit d0b5e3b into devonfw:main Oct 28, 2024
4 checks passed
hohwille added a commit that referenced this pull request Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix macOS specific for Apple MacOS
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

6 participants