[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
> The current version does work for directory saves on windows. > > But if fails on the second save for a zip save: > If you start the editor and save the map "abc" and "abc" already exists it > gets overwritten correctly. > But you can't save again "abc" without restarting the editor (or save once > under another name). > > So the editor itself does put also a lock on the save file after the first > save. > I can confirm this problem - I have created a new bug report so that we can merge this branch: https://bugs.launchpad.net/widelands/+bug/1548932 -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
Review: Approve Now I get it - it's because of file locks. Everything works on Ubuntu now, so I am in favour of merging. Let's solve the problem with not being able to save twice in a new branch. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
I would really like to avoid using different code pathes for windows and linux. The current version does work for directory saves on windows. But if fails on the second save for a zip save: If you start the editor and save the map "abc" and "abc" already exists it gets overwritten correctly. But you can't save again "abc" without restarting the editor (or save once under another name). So the editor itself does put also a lock on the save file after the first save. Why does creating a MapSaver object with new not work on linux? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
Review: Needs Fixing This now works in Windows for the Zip Filesystem, but not for the normal file system. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
Review: Resubmit Ok, it does work again on windows. But i've really no clue what i've done with the last commit. No the file lock on the tmp save is gone and it can be renamed correctly. If this does not break anything on linux i would remove the NOCOMs and merge. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
Continuous integration builds have changed state: Travis build 758. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/110807105. Appveyor build 604. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1509172_map_saving_paths-604. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
Bunnybot encountered an error while working on this merge proposal: Running 'bzr pull --overwrite' failed. Output: ssh_exchange_identification: Connection closed by remote host ConnectionReset reading response for 'BzrDir.open_2.1', retrying ssh_exchange_identification: Connection closed by remote host bzr: ERROR: Connection closed: Unexpected end of message. Please check connectivity and permissions, and report a bug if problems persist. Using saved parent location: bzr+ssh://bazaar.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/ -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
Review: Approve That broke it for Linus, but I have added an #ifndef that should fix it for everybody. I have tested with the zip option on and off. COuld you please test for Windows again to make sure that it's still working? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
Continuous integration builds have changed state: Travis build 709. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/109475085. Appveyor build 558. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1509172_map_saving_paths-558. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
In the error message I got, the path was missing all \. I need to add some more log output so we can get more information as to at what point the path gets messed up. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
Probably i was still uploading...
I've done some debugging, but did not find a solution as of now:
- Click ok in save dialog
- The first string which arrives in ensure_dir_exists is
"C:\data\wl_x64\src\maps" (my binary dir + maps)
- Then the drive letter "C:" get extracted
- Canonalize_name() makes "C:\\Users\\mit\\.widelands\\C:" from this.
- This seems to be done because is_path_absolute("C:") gives false
- This is because path_size < root_size => "C:" <
"C:\\Users\\mit\\.widelands\\"
But why is a path relative just because it is shorter than your widelands home
directory?
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1509172-map-saving-paths.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
Nevermind, the link worked on the 2nd try. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
Review: Needs Fixing => https://widelands.8-schuss.de/Widelands-bzr7830%5bbug-1509172-map-saving-paths%5d-nomusic-win64.exe It does not work: Now ensure_dir_exists() throws an error. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1509172-map-saving-paths. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1509172 in widelands: "editor gives error on saving maps"
https://bugs.launchpad.net/widelands/+bug/1509172
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1509172-map-saving-paths/+merge/286067
I hope that this will fix the map saving bug in Windows. I can reproduce the
original bug, but I need the AppVeyor build to check if this fixes it.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1509172-map-saving-paths into lp:widelands.
=== modified file 'src/editor/ui_menus/editor_main_menu_save_map.cc'
--- src/editor/ui_menus/editor_main_menu_save_map.cc 2016-01-31 10:57:58 +
+++ src/editor/ui_menus/editor_main_menu_save_map.cc 2016-02-15 14:56:39 +
@@ -126,7 +126,7 @@
if (filename == "" && table_.has_selection()) { // Maybe a directory is selected.
complete_filename = filename = maps_data_[table_.get_selected()].filename;
} else {
- complete_filename = curdir_ + "/" + filename;
+ complete_filename = curdir_ + g_fs->file_separator() + filename;
}
if (g_fs->is_directory(complete_filename.c_str()) &&
@@ -155,9 +155,7 @@
if (md.run() == UI::Panel::Returncodes::kOk) {
g_fs->ensure_directory_exists(curdir_);
// create directory
- std::string fullname = curdir_;
- fullname += "/";
- fullname += md.get_dirname();
+ std::string fullname = curdir_ + g_fs->file_separator() + md.get_dirname();
g_fs->make_directory(fullname);
fill_table();
}
@@ -227,16 +225,15 @@
*/
bool MainMenuSaveMap::save_map(std::string filename, bool binary) {
// Make sure that the current directory exists and is writeable.
- g_fs->ensure_directory_exists(curdir_);
+ const std::string current_dir = g_fs->canonicalize_name(curdir_);
+ g_fs->ensure_directory_exists(current_dir);
// OK, first check if the extension matches (ignoring case).
if (!boost::iends_with(filename, WLMF_SUFFIX))
filename += WLMF_SUFFIX;
// append directory name
- std::string complete_filename = curdir_;
- complete_filename += "/";
- complete_filename += filename;
+ std::string complete_filename = current_dir + g_fs->file_separator() + filename;
// Check if file exists. If so, show a warning.
if (g_fs->file_exists(complete_filename)) {
=== modified file 'src/graphic/animation.cc'
--- src/graphic/animation.cc 2016-02-09 08:07:48 +
+++ src/graphic/animation.cc 2016-02-15 14:56:39 +
@@ -114,7 +114,7 @@
const std::string name = sound_effects->get_string("name");
const std::string directory = sound_effects->get_string("directory");
- sound_effect_ = directory + "/" + name;
+ sound_effect_ = directory + g_fs->file_separator() + name;
g_sound_handler.load_fx_if_needed(directory, name, sound_effect_);
}
=== modified file 'src/logic/map_objects/tribes/production_program.cc'
--- src/logic/map_objects/tribes/production_program.cc 2016-02-09 08:07:48 +
+++ src/logic/map_objects/tribes/production_program.cc 2016-02-15 14:56:39 +
@@ -1524,7 +1524,7 @@
bool reached_end;
const std::string& filepath = next_word(parameters, reached_end);
const std::string& filename = next_word(parameters, reached_end);
- name = filepath + "/" + filename;
+ name = filepath + g_fs->file_separator() + filename;
if (!reached_end) {
char * endp;
=== modified file 'src/logic/save_handler.cc'
--- src/logic/save_handler.cc 2015-11-14 15:58:29 +
+++ src/logic/save_handler.cc 2016-02-15 14:56:39 +
@@ -167,9 +167,7 @@
filename += WLGF_SUFFIX;
// Now append directory name
- std::string complete_filename = dir;
- complete_filename += "/";
- complete_filename += filename;
+ std::string complete_filename = dir + g_fs->file_separator() + filename;
return complete_filename;
}
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
