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

Store position and buffer files under the top installation directory. #282

Merged
merged 4 commits into from
Jun 24, 2022

Conversation

igorpeshansky
Copy link
Member

This preserves buffers and pos files across upgrades.

@igorpeshansky igorpeshansky requested a review from a team August 7, 2020 18:32
@google-cla google-cla bot added the cla: yes label Aug 7, 2020
@ghost ghost requested review from jkschulz and removed request for a team August 7, 2020 18:33
windows-installer/setup.nsi Outdated Show resolved Hide resolved
windows-installer/setup.nsi Show resolved Hide resolved
@@ -182,8 +188,10 @@ Section "Install"
; Delete the zip file after extraction.
Delete "$OUTDIR\${ZIP_FILE}"

; Create a directory to store buffer files.
Copy link

Choose a reason for hiding this comment

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

We make the folders before we know if they set the paths to something custom or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, the buffer path is fixed relative to $INSTDIR. Or were you asking about a custom $INSTDIR?
Yes, we do create the folders unconditionally, but it shouldn't matter, because the operation is idempotent.

Choose a reason for hiding this comment

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

What happens if they're set to something custom (i.e. the placeholder is replaced in the config)? Say the default for one of them is defined as default/dir/path but in the config, it's set to custom/dir/path. This looks like it'll only create default/dir/path. Does the custom one get created at another point?

Choose a reason for hiding this comment

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

When installing windows agent, it pops up an installer which you can customize the installation directory. Are you sayin it will be replacing the $INSTDIR otherwise the default will be "$env:UserProfile"?
Another question: where we gave the default value all those parameter after CreateDirectory, e.x. ${MAIN_INSTDIR}?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jkschulz By the time the installer runs, $INSTDIR is already set.

@sophieyfang Sorry, missed this question. The default value is set by the InstallDir directive (line 78).

@qingling128 qingling128 merged commit 90e984f into master Jun 24, 2022
@qingling128 qingling128 deleted the igorpeshansky-win-pos-file-buffer-fix branch June 24, 2022 06:02
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.

None yet

4 participants