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

Fix bin/webpack when ui-classic not bundled #9293

Merged
merged 2 commits into from
Oct 22, 2024
Merged

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Oct 21, 2024

When building rpms, we do not run bundle update in ui-classic (we only run in core)

To build the ui assets, we use rake update:ui which calls ./bin/webpack.
The webpack command needs to know the i18n gem, and asks bundler for that path.
Since bundler has not been installed, this command fails.

The solution is to populate the i18n path in our paths.json file.
Since this path was known when we created paths.json, this is easier to do.

Before

So when running bin/webpack, we get an error when running bundle info:

./bin/webpack

Ruby versions >= 3.2.0 are untested!
The git source https://github.com/ManageIQ/amazon_ssa_support.git is not yet
checked out. Please run `bundle install` before trying to start your application
manageiq-ui-classic/node_modules/webpack-cli/bin/cli.js:93
				throw err;

After

./bin/webpack

[success]

When building rpms, we do not run bundle update in ui-classic (we only run in core)

To build the ui assets, we use `rake update:ui` which calls `./bin/webpack`.
The webpack command needs to know the i18n gem, and asks bundler for that path.
Since bundler has not been installed, this command fails.

The solution is to populate the i18n path in our paths.json file.
Since this path was known when we created paths.json, this is easier to do.

Before
======

So when running bin/webpack, we get an error when running `bundle info`:

```
./bin/webpack

Ruby versions >= 3.2.0 are untested!
The git source https://github.com/ManageIQ/amazon_ssa_support.git is not yet
checked out. Please run `bundle install` before trying to start your application
manageiq-ui-classic/node_modules/webpack-cli/bin/cli.js:93
				throw err;
```

After
=====

```
./bin/webpack

[success]
```
@kbrock kbrock requested a review from a team as a code owner October 21, 2024 20:01
@kbrock
Copy link
Member Author

kbrock commented Oct 21, 2024

update:

  • still was seeing this. Turns out it was the node call that caused the issue.
  • Leaving in the path change, but we can revisit if we want to reduce change.

bin/webpack Outdated
# flag anymore.
has_legacy_openssl = system('node --help | grep -qs "\\--openssl-legacy-provider"')
`node --openssl-legacy-provider --openssl-legacy-provider -v`
Copy link
Member

Choose a reason for hiding this comment

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

What is this change for?

Copy link
Member

Choose a reason for hiding this comment

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

Not that the check here is because not all compilations of node have this option. If you try to call node when the option doesn't exist it blows up.

Copy link
Member Author

@kbrock kbrock Oct 21, 2024

Choose a reason for hiding this comment

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

sorry, added to slack but not here. Will copy here since that is where it belongs.

The old node --help | grep was actually the issue around node pipes. It was failing sporadically. But couldn't nail down why. I could get the command to fail on a bash prompt - so I'm sure this is the issue.

I just put that in so it will fail if is not available and we are looking at $?

I'll fix the typo for double typing the --openssl-legacy-provider

I liked the old code outputting when there were issues.
I could look for other ways to determine if ssl legacy was necessary

Before
======

For some reason, our old method was sporadic

```
node --help | grep -qs "\\--openssl-legacy-provider"
```

node help was throwing a socket close error every few runs locally

After
=====

We are running node a new way that does not throw an exception.
We changed over to back ticks to hide the output from the command
But the $? is updated with the status of the system call and we determine success
@Fryguy Fryguy merged commit 02541a8 into ManageIQ:master Oct 22, 2024
15 checks passed
@Fryguy Fryguy self-assigned this Oct 22, 2024
@kbrock kbrock deleted the paths_json branch October 22, 2024 14:46
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.

2 participants