Skip to content

Rake: update task clean to remove bin and build folders#6032

Merged
matz merged 1 commit intomruby:masterfrom
jbampton:update-rake-task-clean
Jul 31, 2023
Merged

Rake: update task clean to remove bin and build folders#6032
matz merged 1 commit intomruby:masterfrom
jbampton:update-rake-task-clean

Conversation

@jbampton
Copy link
Contributor

No description provided.

@jbampton jbampton requested a review from matz as a code owner July 29, 2023 17:57
@github-actions github-actions bot added the build label Jul 29, 2023
@matz matz merged commit 1bc8ef9 into mruby:master Jul 31, 2023
@jbampton jbampton deleted the update-rake-task-clean branch July 31, 2023 10:21
@dearblue
Copy link
Contributor

The #{MRUBY_ROOT}/build and {MRUBY_ROOT}/bin directories, which are now hardcoded by this PR, can still be replaced.

  • Alternatives to the #{MRUBY_ROOT}/build directory can be specified with the MRuby::Build#build_dir= accessor.
  • The {MRUBY_ROOT}/bin directory can be replaced by the INSTALL_DIR environment variable.
    NOTE: There is no documentation for the INSTALL_DIR environment variable, so I believe it is a hidden feature.

Also, #{MRUBY_ROOT}/build/repos/#{MRuby::Build#name} contains a git clone of the third party mrbgems.
The git repository is also removed by rake clean with this PR, so people who are in the middle of modifying other people's code may have trouble with rake clean.
NOTE: The MRuby::Build#gem_clone_dir= accessor allows you to specify an alternative, but I believe it is a hidden feature that is not documented.

Therefore, I will create a PR to revert to.

In my opinion, if the content of this PR is needed, it would be better to define it as a different task than rake clean or rake deep_clean.

@matz
Copy link
Member

matz commented Aug 21, 2023

I think it's worth removing the default build and bin directories in the cleaning task. I think it should be in the deep_clean task.

@dearblue why do you think it should be a different task than existing clean tasks?

@dearblue
Copy link
Contributor

This is because the current deep_clean task only removes third-party GEMS directories related to the build target based on the "Build configuration file" at the time.
Unrelated GEMS directories will be kept intact (on a per-build-target basis).

@matz
Copy link
Member

matz commented Aug 22, 2023

The original meaning of deep_clean was “to clean everything generated during build”, not only in the last build, but in all builds. So it's natural to remove bin and build in the task, as default build target directories. I know some files may be generated outside those directories (according to configurations), but it's better than nothing.

Some software uses distclean task for the purpose, but I see no specific reason for separate tasks.

@dearblue
Copy link
Contributor

I understood. If that was the original purpose of deep_clean, I have no disagreement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants