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

Stop if JQ is not installed #125

Open
wants to merge 1 commit into
base: 6.4
Choose a base branch
from
Open

Stop if JQ is not installed #125

wants to merge 1 commit into from

Conversation

amenk
Copy link
Contributor

@amenk amenk commented Nov 30, 2021

Running the build without JQ will only lead to more problems, so it is better to fail early

Running the build without JQ will only lead to more problems, so it is better to fail early
@keulinho
Copy link
Contributor

keulinho commented Dec 1, 2021

What problems do you experience exactly? Is it all related to the npm dependencies of plugins not being installed because of the missing jq?

@amenk
Copy link
Contributor Author

amenk commented Dec 1, 2021

Yes exactly. We have a plugins.json but the build failed because (we believe) the npm install did not run for the plugins:

ERROR in /web/vendor/store.shopware.com/swagpaypal/src/Resources/app/storefront/src/swag-paypal.abstract-buttons.js
Module not found: Error: Can't resolve '@paypal/paypal-js' in '/web/vendor/store.shopware.com/swagpaypal/src/Resources/app/storefront/src'

So I think it is better to enforce the presence of jq to avoid harder-to-debug errors afterwards.

What do you think?

@keulinho
Copy link
Contributor

keulinho commented Dec 1, 2021

I think that depends, as long as you don't have plugins that need additional npm dependenies you don't need jq, but in order to be able to detect if there are plugins with npm dependencies jq needs to be installed.

@amenk
Copy link
Contributor Author

amenk commented Dec 1, 2021

Okay, so maybe we should at least improve the warning. Anyways, I am a fan of things failing early, so I still would say it's fine to just require "jq".

@amenk
Copy link
Contributor Author

amenk commented Oct 18, 2022

We could also add a SKIP_DEPENDENCY_CHECK flag one could set who is sure that there are no dependencies which would skip the whole jq part.
Still a fan of fail-early and avoiding hard-to-debug issues :-)

@Isengo1989
Copy link

I agree, this can be really time consuming and frustrating (for beginners) - I think a flag should be enough.

@keulinho what is your fear in adding the exit? Hosters that don't provide the extension or users that get stuck there?

@amenk
Copy link
Contributor Author

amenk commented Apr 20, 2023

I guess we should do that change for 6.5 somewhere at https://github.com/shopware/recipes/blob/main/shopware/storefront/6.4/bin/build-storefront.sh ?

Shall we go head with a SKIP_DEPENDENCY_CHECK flag?

@matheusgontijo
Copy link

I also spent a good amount of time figuring out that issue... That would be really really great to have it merged to the project. At least developers will quickly find out that there is a dependency required, instead of just silently erroring out.

By the way, jq is not highly used on servers out there.

@ChristianOellers
Copy link

It would be nice to have these added to the list of requirements. We also had the issue with our hoster, and needed to convince them to install it. So if that went into the system requirements or recommendations, this could be a place to tell them or check by ourselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants