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

update webpack to v3.0 #1693

Merged
merged 1 commit into from
Jul 12, 2017
Merged

update webpack to v3.0 #1693

merged 1 commit into from
Jul 12, 2017

Conversation

ohadlevy
Copy link
Member

No description provided.

@dLobatog
Copy link
Member

http://koji.katello.org/kojifiles/work/tasks/8155/8155/build.log fails because there's no npm:

+ npm cache add --cache ./npm_cache /builddir/build/SOURCES/webpack-3.0.0.tgz
/var/tmp/rpm-tmp.ywlyav: line 32: npm: command not found

@ohadlevy
Copy link
Member Author

@dLobatog any idea why there is no npm?

@ohadlevy
Copy link
Member Author

[test]

@ohadlevy
Copy link
Member Author

@ehelms the tests fails because of it cant find packages introduced in this ps, is there a way to test all changes in one pr? or must i break it down to a PR per package? thanks.

@ehelms
Copy link
Member

ehelms commented Jun 26, 2017

@ohadlevy Traditionally these kinds of changes have been done in a PR like this one. But it puts a burden on the reviewer to check it out locally, run mockchain builds to ensure everything passes. So while I prefer smaller individual PRs to address, this is fine but will take me a (or another maintainer) a bit longer to get to.

@ehelms
Copy link
Member

ehelms commented Jul 4, 2017

@ohadlevy How many packages are updated in this PR? It's really hard to tell with the number of files changed what all has been changed and waht is going to need updating and what are new packages. If you could either list that out or split this up it would help a ton.

@ehelms
Copy link
Member

ehelms commented Jul 5, 2017

@ohadlevy there appear to be some actual build errors when looking at (http://ci.theforeman.org/job/packaging_test_pull_request/2329/) that aren't in root log (typically the case for when dependent packages are missing. For example, I see (http://koji.katello.org/koji/taskinfo?taskID=8560) failing in the build.log with what appears to be a legitimate error. Can you check these and confirm?

@mmoll
Copy link

mmoll commented Jul 10, 2017

@ohadlevy what's the status here?

@ohadlevy
Copy link
Member Author

@ehelms I'm not seeing the actual error - does koji no longer keeps it?

@ehelms
Copy link
Member

ehelms commented Jul 10, 2017

@ohadlevy it will keep scratch builds for a short time, we can re-run it

@ehelms
Copy link
Member

ehelms commented Jul 10, 2017

[test]

@ehelms
Copy link
Member

ehelms commented Jul 11, 2017

I had to make the following changes to get the builds passing:

diff --git a/nodejs-extract-text-webpack-plugin/nodejs-bundle-extract-text-webpack-plugin.spec b/nodejs-extract-text-webpack-plugin/nodejs-extract-text-webpack-plugin.spec
similarity index 92%
rename from nodejs-extract-text-webpack-plugin/nodejs-bundle-extract-text-webpack-plugin.spec
rename to nodejs-extract-text-webpack-plugin/nodejs-extract-text-webpack-plugin.spec
index 5ebdea2..a553bc3 100644
--- a/nodejs-extract-text-webpack-plugin/nodejs-bundle-extract-text-webpack-plugin.spec
+++ b/nodejs-extract-text-webpack-plugin/nodejs-extract-text-webpack-plugin.spec
@@ -51,8 +51,8 @@ Provides: bundled-npm(fast-deep-equal) = 0.1.0
 Provides: bundled-npm(json-stable-stringify) = 1.0.1
 Provides: bundled-npm(co) = 4.6.0
 Provides: bundled-npm(jsonify) = 0.0.0
-AutoReq: no 
-AutoProv: no 
+AutoReq: no
+AutoProv: no
 
 
 %description
@@ -72,13 +72,10 @@ npm install --cache-min Infinity --cache . --no-optional --global-style true %{n
 %install
 mkdir -p %{buildroot}%{nodejs_sitelib}/%{npm_name}
 cd node_modules/extract-text-webpack-plugin
-cp -pfr CHANGELOG.md ExtractedModule.js LICENSE OrderUndefinedError.js README.md index.js loader.js package.json schema node_modules %{buildroot}%{nodejs_sitelib}/%{npm_name}
+cp -pfr CHANGELOG.md ExtractedModule.js OrderUndefinedError.js README.md index.js loader.js package.json schema node_modules %{buildroot}%{nodejs_sitelib}/%{npm_name}
 cp -pf CHANGELOG.md README.md ../../
 # If any binaries are included, symlink them to bindir here
 
-
-%nodejs_symlink_deps
-
 %if 0%{?enable_tests}
 %check
 %{nodejs_symlink_deps} --check
@@ -88,7 +85,6 @@ cp -pf CHANGELOG.md README.md ../../
 %files
 %{nodejs_sitelib}/%{npm_name}
 
-%doc LICENSE
 %doc CHANGELOG.md
 %doc README.md
 
diff --git a/nodejs-fbjs/nodejs-bundle-fbjs.spec b/nodejs-fbjs/nodejs-fbjs.spec
similarity index 90%
rename from nodejs-fbjs/nodejs-bundle-fbjs.spec
rename to nodejs-fbjs/nodejs-fbjs.spec
index 9c5ed42..453f187 100644
--- a/nodejs-fbjs/nodejs-bundle-fbjs.spec
+++ b/nodejs-fbjs/nodejs-fbjs.spec
@@ -1,6 +1,3 @@
-# FIXME
-# Remove nodejs_symlink_deps if installing bundled module with NPM
-
 %global npm_name fbjs
 %global enable_tests 0
 
@@ -47,8 +44,8 @@ Provides: bundled-npm(node-fetch) = 1.7.1
 Provides: bundled-npm(is-stream) = 1.1.0
 Provides: bundled-npm(encoding) = 0.1.12
 Provides: bundled-npm(iconv-lite) = 0.4.18
-AutoReq: no 
-AutoProv: no 
+AutoReq: no
+AutoProv: no
 
 
 %description
@@ -68,13 +65,10 @@ npm install --cache-min Infinity --cache . --no-optional --global-style true %{n
 %install
 mkdir -p %{buildroot}%{nodejs_sitelib}/%{npm_name}
 cd node_modules/fbjs
-cp -pfr CHANGELOG.md LICENSE PATENTS README.md flow index.js lib module-map.json package.json node_modules %{buildroot}%{nodejs_sitelib}/%{npm_name}
+cp -pfr CHANGELOG.md PATENTS README.md flow index.js lib module-map.json package.json node_modules %{buildroot}%{nodejs_sitelib}/%{npm_name}
 cp -pf CHANGELOG.md README.md ../../
 # If any binaries are included, symlink them to bindir here
 
-
-%nodejs_symlink_deps
-
 %if 0%{?enable_tests}
 %check
 %{nodejs_symlink_deps} --check
@@ -84,7 +78,6 @@ cp -pf CHANGELOG.md README.md ../../
 %files
 %{nodejs_sitelib}/%{npm_name}
 
-%doc LICENSE
 %doc CHANGELOG.md
 %doc README.md
 
diff --git a/nodejs-stats-webpack-plugin/nodejs-bundle-stats-webpack-plugin.spec b/nodejs-stats-webpack-plugin/nodejs-stats-webpack-plugin.spec
similarity index 84%
rename from nodejs-stats-webpack-plugin/nodejs-bundle-stats-webpack-plugin.spec
rename to nodejs-stats-webpack-plugin/nodejs-stats-webpack-plugin.spec
index 6d23add..aae37a6 100644
--- a/nodejs-stats-webpack-plugin/nodejs-bundle-stats-webpack-plugin.spec
+++ b/nodejs-stats-webpack-plugin/nodejs-stats-webpack-plugin.spec
@@ -1,6 +1,3 @@
-# FIXME
-# Remove nodejs_symlink_deps if installing bundled module with NPM
-
 %global npm_name stats-webpack-plugin
 %global enable_tests 0
 
@@ -21,8 +18,8 @@ ExclusiveArch: %{nodejs_arches} noarch
 
 Provides: bundled-npm(stats-webpack-plugin) = 0.6.1
 Provides: bundled-npm(lodash) = 4.17.4
-AutoReq: no 
-AutoProv: no 
+AutoReq: no
+AutoProv: no
 
 
 %description
@@ -42,13 +39,10 @@ npm install --cache-min Infinity --cache . --no-optional --global-style true %{n
 %install
 mkdir -p %{buildroot}%{nodejs_sitelib}/%{npm_name}
 cd node_modules/stats-webpack-plugin
-cp -pfr LICENSE README.md index.js package.json node_modules %{buildroot}%{nodejs_sitelib}/%{npm_name}
+cp -pfr README.md index.js package.json node_modules %{buildroot}%{nodejs_sitelib}/%{npm_name}
 cp -pf README.md ../../
 # If any binaries are included, symlink them to bindir here
 
-
-%nodejs_symlink_deps
-
 %if 0%{?enable_tests}
 %check
 %{nodejs_symlink_deps} --check
@@ -58,7 +52,6 @@ cp -pf README.md ../../
 %files
 %{nodejs_sitelib}/%{npm_name}
 
-%doc LICENSE
 %doc README.md
 
 %changelog
diff --git a/nodejs-webpack/nodejs-bundle-webpack.spec b/nodejs-webpack/nodejs-webpack.spec
similarity index 99%
rename from nodejs-webpack/nodejs-bundle-webpack.spec
rename to nodejs-webpack/nodejs-webpack.spec
index c00e921..5e7fa94 100644
--- a/nodejs-webpack/nodejs-bundle-webpack.spec
+++ b/nodejs-webpack/nodejs-webpack.spec
@@ -1,6 +1,3 @@
-# FIXME
-# Remove nodejs_symlink_deps if installing bundled module with NPM
-
 %global npm_name webpack
 %global enable_tests 0
 
@@ -629,8 +626,8 @@ Provides: bundled-npm(ecc-jsbn) = 0.1.1
 Provides: bundled-npm(tweetnacl) = 0.14.5
 Provides: bundled-npm(is-number) = 3.0.0
 Provides: bundled-npm(kind-of) = 4.0.0
-AutoReq: no 
-AutoProv: no 
+AutoReq: no
+AutoProv: no
 
 
 %description
@@ -650,17 +647,14 @@ npm install --cache-min Infinity --cache . --no-optional --global-style true %{n
 %install
 mkdir -p %{buildroot}%{nodejs_sitelib}/%{npm_name}
 cd node_modules/webpack
-cp -pfr LICENSE README.md bin buildin hot lib package.json schemas web_modules node_modules %{buildroot}%{nodejs_sitelib}/%{npm_name}
+cp -pfr README.md bin buildin hot lib package.json schemas web_modules node_modules %{buildroot}%{nodejs_sitelib}/%{npm_name}
 cp -pf README.md ../../
 # If any binaries are included, symlink them to bindir here
-mkdir -p %{buildroot}%{nodejs_sitelib}/${npm_name}/bin 
-mkdir -p %{buildroot}%{_bindir}/ 
+mkdir -p %{buildroot}%{nodejs_sitelib}/${npm_name}/bin
+mkdir -p %{buildroot}%{_bindir}/
 install -p -D -m0755 bin/webpack.js %{buildroot}%{nodejs_sitelib}/%{npm_name}/bin/webpack.js
 ln -sf %{nodejs_sitelib}/%{npm_name}/bin/webpack.js %{buildroot}%{_bindir}/webpack.js
 
-
-%nodejs_symlink_deps
-
 %if 0%{?enable_tests}
 %check
 %{nodejs_symlink_deps} --check
@@ -670,7 +664,6 @@ ln -sf %{nodejs_sitelib}/%{npm_name}/bin/webpack.js %{buildroot}%{_bindir}/webpa
 %files
 %{nodejs_sitelib}/%{npm_name}
 %{_bindir}/webpack.js
-%doc LICENSE
 %doc README.md
 
 %changelog

@ohadlevy
Copy link
Member Author

@ehelms I've applied your changes.

@ehelms
Copy link
Member

ehelms commented Jul 12, 2017

[test]

@ehelms
Copy link
Member

ehelms commented Jul 12, 2017

Thanks @ohadlevy . The webpack package is passing so I expect the plugins to also pass as they did locally. Merging and will build.

@ehelms ehelms merged commit 49602d6 into theforeman:rpm/develop Jul 12, 2017
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.

5 participants