Stop packaging node_modules in release tarballs #15273

techknowlogick · 2021-04-04T09:03:09Z

silverwind commented

2021-04-04 09:03:09 +00:00

(Migrated from github.com)

Don't package node_modules in tarballs, they are not cross-platform anymore and npm cache should not be messed with directly. Instead, require an internet connection to rebuild the UI, which is not necessary in the general use case because prebuilt UI files are shipped in the public directory.
Simplify the fomantic build and make the target phony. We don't need anything more for something that is rarely ran.
Use regular tar again to build tarballs and add variable for excludes
Disable annoying npm update notifications

Fixes: https://github.com/go-gitea/gitea/pull/14578
Fixes: https://github.com/go-gitea/gitea/pull/15256
Fixes: https://github.com/go-gitea/gitea/pull/15262

- Don't package node_modules in tarballs, they are not cross-platform anymore and npm cache should not be messed with directly. Instead, require an internet connection to rebuild the UI, which is not necessary in the general use case because prebuilt UI files are shipped in the public directory. - Simplify the fomantic build and make the target phony. We don't need anything more for something that is rarely ran. - Use regular tar again to build tarballs and add variable for excludes - Disable annoying npm update notifications Fixes: https://github.com/go-gitea/gitea/pull/14578 Fixes: https://github.com/go-gitea/gitea/pull/15256 Fixes: https://github.com/go-gitea/gitea/pull/15262

👍 1

CL-Jeremy (Migrated from github.com) requested changes 2021-04-04 11:20:16 +00:00

Makefile

CL-Jeremy (Migrated from github.com) commented

2021-04-04 11:19:25 +00:00

	$(eval EXCL := --exclude=$(shell tar --help | grep -q bsdtar && echo "^" )./)
	tar $(addprefix $(EXCL),$(TAR_EXCLUDES)) -czf $(DIST)/release/gitea-src-$(VERSION).tar.gz .

```suggestion $(eval EXCL := --exclude=$(shell tar --help | grep -q bsdtar && echo "^" )./) tar $(addprefix $(EXCL),$(TAR_EXCLUDES)) -czf $(DIST)/release/gitea-src-$(VERSION).tar.gz . ```

CL-Jeremy (Migrated from github.com) reviewed 2021-04-04 11:24:09 +00:00

Makefile

CL-Jeremy (Migrated from github.com) commented

2021-04-04 11:24:09 +00:00

Ugh, GitHub converts between tabs and spaces automatically in commit diff view, but doesn't seem to touch the lines added in suggestion...

silverwind (Migrated from github.com) reviewed 2021-04-04 20:47:01 +00:00

Makefile

silverwind (Migrated from github.com) commented

2021-04-04 20:47:01 +00:00

Is this related to a difference in the exclude syntax? The current syntax produces similar sized tarballs for me, using tar (GNU tar) 1.34 and bsdtar 3.3.2, so I'm not sure it's necessary.

Is this related to a difference in the exclude syntax? The current syntax produces similar sized tarballs for me, using `tar (GNU tar) 1.34` and `bsdtar 3.3.2`, so I'm not sure it's necessary.

CL-Jeremy (Migrated from github.com) reviewed 2021-04-04 20:58:07 +00:00

Makefile

CL-Jeremy (Migrated from github.com) commented

2021-04-04 20:58:06 +00:00

That's because node_modules isn't included anymore, so all those dist subfolders living there are also spared in both cases. If any folder gets named dist in the future they will be excluded by bsdtar but included by GNU tar. This discovery was the origin of all hassles around bsdtar...

That's because `node_modules` isn't included anymore, so all those `dist` subfolders living there are also spared in both cases. If any folder gets named `dist` in the future they will be excluded by `bsdtar` but included by GNU tar. This discovery was the origin of all hassles around `bsdtar`...

silverwind (Migrated from github.com) reviewed 2021-04-04 21:12:50 +00:00

Makefile

silverwind (Migrated from github.com) commented

2021-04-04 21:12:50 +00:00

Thanks, I'll add a comment why its necessary.

6543 (Migrated from github.com) reviewed 2021-04-06 09:50:27 +00:00

Makefile

6543 (Migrated from github.com) commented

2021-04-06 09:50:27 +00:00

TAR_EXCLUDES := .git data node_modules $(FOMANTIC_WORK_DIR)/node_modules $(DIST) $(MAKE_EVIDENCE_DIR) $(AIR_TMP_DIR)

```suggestion TAR_EXCLUDES := .git data node_modules $(FOMANTIC_WORK_DIR)/node_modules $(DIST) $(MAKE_EVIDENCE_DIR) $(AIR_TMP_DIR) ```

silverwind (Migrated from github.com) reviewed 2021-04-06 09:58:57 +00:00

Makefile

silverwind (Migrated from github.com) commented

2021-04-06 09:58:56 +00:00

I guess we can put even more such folders that may be present in a development setup using the default paths, like indexersand queues directories. Thought it's really a non-issue because CI builds from a clean slate anyways.

I guess we can put even more such folders that may be present in a development setup using the default paths, like `indexers`and `queues` directories. Thought it's really a non-issue because CI builds from a clean slate anyways.