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

make distclean removes version-controlled files #21163

Closed
Trott opened this issue Jun 6, 2018 · 3 comments
Closed

make distclean removes version-controlled files #21163

Trott opened this issue Jun 6, 2018 · 3 comments
Labels
build Issues and PRs related to build files or the CI.

Comments

@Trott
Copy link
Member

Trott commented Jun 6, 2018

  • Version: 11.0.0-pre (master)
  • Platform: POSIX
  • Subsystem: build
$ git status
On branch master
Your branch is up-to-date with 'origin/master'.

nothing to commit, working tree clean
$ make distclean
rm -f -r out
rm -f config.gypi icu_config.gypi config_fips.gypi
rm -f config.mk
rm -f -r node node_g
rm -f -r node_modules
rm -f -r deps/icu
rm -f -r deps/icu4c*.tgz deps/icu4c*.zip deps/icu-tmp
rm -f node-v11.0.0-darwin-x64.tar.* node-v11.0.0.tar.*
rm -f -r deps/v8/testing/gmock
$ git status
On branch master
Your branch is up-to-date with 'origin/master'.

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	deleted:    deps/v8/testing/gmock/BUILD.gn
	deleted:    deps/v8/testing/gmock/OWNERS
	deleted:    deps/v8/testing/gmock/include/DEPS
	deleted:    deps/v8/testing/gmock/include/gmock/gmock-actions.h
	deleted:    deps/v8/testing/gmock/include/gmock/gmock-generated-function-mockers.h
	deleted:    deps/v8/testing/gmock/include/gmock/gmock-matchers.h
	deleted:    deps/v8/testing/gmock/include/gmock/gmock.h

no changes added to commit (use "git add" and/or "git commit -a")
$ 

I'm not sure if the right solution is to change the Makefile so it doesn't delete those files, or if the right solution is to remove those files from version control. I'm guessing it's the former, but maybe they get generated? (If I had compilation working on my machine right now, I'd test/check.)

@Trott Trott added the build Issues and PRs related to build files or the CI. label Jun 6, 2018
@Trott
Copy link
Member Author

Trott commented Jun 6, 2018

@nodejs/build

@Trott Trott changed the title make distclean removes version controlled files make distclean removes version-controlled files Jun 6, 2018
@targos
Copy link
Member

targos commented Jun 6, 2018

These files were added to version control by the V8 project in v8/v8@539e907 (V8 6.7) so yes, changing the Makefile seems like the right thing to do.

@nodejs/v8-update

@ryzokuken
Copy link
Contributor

I'd side with the former too. They're version-controlled files that were updated recently.

Making a PR.

ryzokuken added a commit to ryzokuken/node that referenced this issue Jun 6, 2018
Stop `make distclean` from deleting files in the `deps/v8/testing/gmock`
folder, thus avoiding deleting version-controlled files important for
v8.

Fixes: nodejs#21163
@Trott Trott closed this as completed in 2237a8e Jun 9, 2018
targos pushed a commit that referenced this issue Jun 10, 2018
Stop `make distclean` from deleting files in the `deps/v8/testing/gmock`
folder, thus avoiding deleting version-controlled files important for
v8.

Fixes: #21163

PR-URL: #21164
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this issue Jun 13, 2018
Stop `make distclean` from deleting files in the `deps/v8/testing/gmock`
folder, thus avoiding deleting version-controlled files important for
v8.

Fixes: #21163

PR-URL: #21164
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants