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

fix: 沙箱应用进程会根据 app_desc 改变 #1711

Open
wants to merge 6 commits into
base: builder-stack
Choose a base branch
from

Conversation

SheepSheepChen
Copy link
Collaborator

No description provided.

@narasux narasux changed the title fix:沙箱应用进程会根据 app_desc 改变 fix: 沙箱应用进程会根据 app_desc 改变 Nov 7, 2024
@@ -199,6 +205,12 @@ func parseDeployStepOpts(oldDir, newDir string) *deployStepOpts {
eq, err := utils.SortedCompareFile(oldFilePath, newFilePath)
if err != nil || !eq {
rebuild = true
// metaDataRebuildFiles 中的文件被修改时, 需要删除 metadata.toml, 以便生成最新的 metadata.toml
if slices.Contains(metaDataRebuildFiles, fileName) {
if err := os.Remove(path.Join(devsandbox.DefaultLayersDir, "config", "metadata.toml")); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

直接用 launch.GetMetadataFilePath(devsandbox.DefaultLayersDir) 获取 metadata.toml 路径

Copy link
Collaborator

@jamesgetx jamesgetx Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的逻辑很奇怪,parseDeployStepOpts 不应该有删除文件的副动作。正常情况下,我们不应该去维护 metadata.toml。建议搞清楚原因再实现

buildDependentFiles := []string{"requirements.txt", "Aptfile", "runtime.txt", "Procfile"}
func parseDeployStepOpts(oldDir, newDir string) (*deployStepOpts, error) {
buildDependentFiles := []string{"requirements.txt", "Aptfile", "runtime.txt", "Procfile", "app_desc.yaml"}
metaDataRebuildFiles := []string{"Procfile", "app_desc.yaml"}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebuild 会受到 app_desc.yaml 哪个配置的影响?

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.

2 participants