Eeschema: fix multiple sheet file name bugs.

Add missing check for root sheet when searching sheet hierarchies for
already loaded schematics.  This prevents the root sheet from being
omitted when adding new sheets using the root sheet file name.

CHANGED: Make file name tests case sensitive so that schematic sheet
file names on non-Windows systems can be uses as expected.

Warn users when attempting to use schematic file names that only vary
by case sensitivity that doing so will result in a project that is not
portable to Windows.

Fixes lp:1843415

https://bugs.launchpad.net/kicad/+bug/1843415
(cherry picked from commit d4cea0f2b7)
This commit is contained in:
Wayne Stambaugh 2019-11-17 11:43:05 -05:00
parent ef80642dec
commit d7605f1449
7 changed files with 118 additions and 5 deletions

View File

@ -357,6 +357,7 @@ static const wxString RepeatStepXEntry = "RepeatStepX";
static const wxString RepeatStepYEntry = "RepeatStepY"; static const wxString RepeatStepYEntry = "RepeatStepY";
static const wxString RepeatLabelIncrementEntry = "RepeatLabelIncrement"; static const wxString RepeatLabelIncrementEntry = "RepeatLabelIncrement";
static const wxString ShowIllegalSymboLibDialog = "ShowIllegalSymbolLibDialog"; static const wxString ShowIllegalSymboLibDialog = "ShowIllegalSymbolLibDialog";
static const wxString showSheetFileNameCaseSensitivityDlg = "ShowSheetFileNameCaseSensitivityDlg";
// Library editor wxConfig entry names. // Library editor wxConfig entry names.
static const wxChar defaultLibWidthEntry[] = wxT( "LibeditLibWidth" ); static const wxChar defaultLibWidthEntry[] = wxT( "LibeditLibWidth" );
@ -401,7 +402,9 @@ PARAM_CFG_ARRAY& SCH_EDIT_FRAME::GetConfigurationSettings()
-10, +10 ) ); -10, +10 ) );
m_configSettings.push_back( new PARAM_CFG_BOOL( true, ShowIllegalSymboLibDialog, m_configSettings.push_back( new PARAM_CFG_BOOL( true, ShowIllegalSymboLibDialog,
&m_showIllegalSymbolLibDialog, true ) ); &m_showIllegalSymbolLibDialog, true ) );
m_configSettings.push_back( new PARAM_CFG_BOOL( true, showSheetFileNameCaseSensitivityDlg,
&m_showSheetFileNameCaseSensitivityDlg,
true ) );
return m_configSettings; return m_configSettings;
} }
@ -547,6 +550,7 @@ void SCH_EDIT_FRAME::SaveSettings( wxConfigBase* aCfg )
record.Replace( wxT(" "), wxT(" "), true ); // double space to single record.Replace( wxT(" "), wxT(" "), true ); // double space to single
aCfg->Write( FieldNamesEntry, record ); aCfg->Write( FieldNamesEntry, record );
aCfg->Write( showSheetFileNameCaseSensitivityDlg, m_showSheetFileNameCaseSensitivityDlg );
} }

View File

@ -387,6 +387,7 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ):
m_undoItem = NULL; m_undoItem = NULL;
m_hasAutoSave = true; m_hasAutoSave = true;
m_showIllegalSymbolLibDialog = true; m_showIllegalSymbolLibDialog = true;
m_showSheetFileNameCaseSensitivityDlg = true;
m_FrameSize = ConvertDialogToPixels( wxSize( 500, 350 ) ); // default in case of no prefs m_FrameSize = ConvertDialogToPixels( wxSize( 500, 350 ) ); // default in case of no prefs
m_AboutTitle = "Eeschema"; m_AboutTitle = "Eeschema";

View File

@ -151,6 +151,7 @@ private:
bool m_autoplaceAlign; ///< align autoplaced fields to the grid bool m_autoplaceAlign; ///< align autoplaced fields to the grid
bool m_footprintPreview; ///< whether to show footprint previews bool m_footprintPreview; ///< whether to show footprint previews
bool m_showIllegalSymbolLibDialog; bool m_showIllegalSymbolLibDialog;
bool m_showSheetFileNameCaseSensitivityDlg;
/// An index to the last find item in the found items list #m_foundItems. /// An index to the last find item in the found items list #m_foundItems.
int m_foundItemIndex; int m_foundItemIndex;
@ -985,6 +986,19 @@ private:
*/ */
bool importFile( const wxString& aFileName, int aFileType ); bool importFile( const wxString& aFileName, int aFileType );
/**
* Check \a aSchematicFileName for a potential file name case sensitivity clashes.
*
* On platforms where file names are case sensitive, it is possible to schematic sheet
* file names that would cause issues on platforms where file name are case insensitive.
* File names foo.sch and Foo.sch are unique files on Linux and MacOS but on Windows
* this would result in a broken schematic.
*
* @param aSchematicFileName is the absolute path and file name of the file to test.
* @return true if the user accepts the potential file name clase risk.
*/
bool allowCaseSensitiveFileNameClashes( const wxString& aSchematicFileName );
public: public:
/** /**
* Command event handler to change a text type to another one. * Command event handler to change a text type to another one.

View File

@ -1619,6 +1619,29 @@ bool SCH_SCREENS::HasSchematic( const wxString& aSchematicFileName )
} }
bool SCH_SCREENS::CanCauseCaseSensitivityIssue( const wxString& aSchematicFileName ) const
{
wxFileName lhs;
wxFileName rhs = aSchematicFileName;
wxCHECK( rhs.IsAbsolute(), false );
for( const SCH_SCREEN* screen : m_screens )
{
lhs = screen->GetFileName();
if( lhs.GetPath() != rhs.GetPath() )
continue;
if( lhs.GetName().CmpNoCase( rhs.GetName() )
&& lhs.GetName() != rhs.GetName() )
return true;
}
return false;
}
void SCH_SCREENS::BuildClientSheetPathList() void SCH_SCREENS::BuildClientSheetPathList()
{ {
SCH_SHEET_LIST sheetList( g_RootSheet ); SCH_SHEET_LIST sheetList( g_RootSheet );

View File

@ -639,6 +639,19 @@ public:
void BuildClientSheetPathList(); void BuildClientSheetPathList();
/**
* Check \a aSchematicFileName for a potential file name case sensitivity issue.
*
* On platforms where file names are case sensitive, it is possible to schematic sheet
* file names that would cause issues on platforms where file name are case insensitive.
* File names foo.sch and Foo.sch are unique files on Linux and MacOS but on Windows
* this would result in a broken schematic.
*
* @param aSchematicFileName is the absolute path and file name of the file to test.
* @return true if \a aSchematicFileName would cause an issue.
*/
bool CanCauseCaseSensitivityIssue( const wxString& aSchematicFileName ) const;
private: private:
void addScreenToList( SCH_SCREEN* aScreen ); void addScreenToList( SCH_SCREEN* aScreen );
void buildScreenList( SCH_SHEET* aSheet); void buildScreenList( SCH_SHEET* aSheet);

View File

@ -578,18 +578,36 @@ int SCH_SHEET::ComponentCount()
bool SCH_SHEET::SearchHierarchy( const wxString& aFilename, SCH_SCREEN** aScreen ) bool SCH_SHEET::SearchHierarchy( const wxString& aFilename, SCH_SCREEN** aScreen )
{ {
SCH_SHEET* sheet = nullptr;
SCH_SCREEN* screen = nullptr;
if( m_screen ) if( m_screen )
{ {
// Only check the root sheet once and don't recurse.
if( !GetParent() )
{
sheet = this;
screen = m_screen;
if( screen && screen->GetFileName().Cmp( aFilename ) == 0 )
{
*aScreen = screen;
return true;
}
}
EDA_ITEM* item = m_screen->GetDrawItems(); EDA_ITEM* item = m_screen->GetDrawItems();
while( item ) while( item )
{ {
if( item->Type() == SCH_SHEET_T ) if( item->Type() == SCH_SHEET_T )
{ {
SCH_SHEET* sheet = (SCH_SHEET*) item; // Must use the screen's path (which is always absolute) rather than the
// sheet's (which could be relative).
sheet = static_cast< SCH_SHEET* >( item );
screen = sheet->m_screen;
if( sheet->m_screen if( screen && screen->GetFileName().Cmp( aFilename ) == 0 )
&& sheet->m_screen->GetFileName().CmpNoCase( aFilename ) == 0 )
{ {
*aScreen = sheet->m_screen; *aScreen = sheet->m_screen;
return true; return true;

View File

@ -574,6 +574,9 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy,
if( aSheet->GetScreen() == NULL ) // New sheet. if( aSheet->GetScreen() == NULL ) // New sheet.
{ {
if( !allowCaseSensitiveFileNameClashes( newFilename ) )
return false;
if( useScreen || loadFromFile ) // Load from existing file. if( useScreen || loadFromFile ) // Load from existing file.
{ {
clearAnnotation = true; clearAnnotation = true;
@ -605,6 +608,9 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy,
isExistingSheet = true; isExistingSheet = true;
if( !allowCaseSensitiveFileNameClashes( newFilename ) )
return false;
// Changing the filename of a sheet can modify the full hierarchy structure // Changing the filename of a sheet can modify the full hierarchy structure
// and can be not always undoable. // and can be not always undoable.
// So prepare messages for user notifications: // So prepare messages for user notifications:
@ -627,7 +633,7 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy,
wxString oldFilename = aSheet->GetScreen()->GetFileName(); wxString oldFilename = aSheet->GetScreen()->GetFileName();
oldFilename.Replace( wxT( "\\" ), wxT( "/" ) ); oldFilename.Replace( wxT( "\\" ), wxT( "/" ) );
if( newFilename.CmpNoCase( oldFilename ) != 0 ) if( newFilename.Cmp( oldFilename ) != 0 )
{ {
// Sheet file name changes cannot be undone. // Sheet file name changes cannot be undone.
isUndoable = false; isUndoable = false;
@ -974,3 +980,37 @@ void SCH_EDIT_FRAME::MirrorSheet( SCH_SHEET* aSheet, bool aFromXaxis )
GetCanvas()->GetView()->Update( aSheet ); GetCanvas()->GetView()->Update( aSheet );
OnModify(); OnModify();
} }
bool SCH_EDIT_FRAME::allowCaseSensitiveFileNameClashes( const wxString& aSchematicFileName )
{
wxString msg;
SCH_SCREENS screens;
wxFileName fn = aSchematicFileName;
wxCHECK( fn.IsAbsolute(), false );
if( m_showSheetFileNameCaseSensitivityDlg
&& screens.CanCauseCaseSensitivityIssue( aSchematicFileName ) )
{
msg.Printf( _( "The file name \"%s\" can cause issues with an existing file name\n"
"already defined in the schematic on systems that support case\n"
"insensitive file names. This will cause issues if you copy this\n"
"project to an operating system that supports case insensitive file\n"
"names.\n\nDo you wish to continue?" ),
fn.GetName() );
wxRichMessageDialog dlg( this, msg, _( "Warning" ),
wxYES_NO | wxNO_DEFAULT | wxICON_QUESTION );
dlg.ShowCheckBox( _( "Do not show this message again." ) );
dlg.SetYesNoLabels( wxMessageDialog::ButtonLabel( _( "Create New Sheet" ) ),
wxMessageDialog::ButtonLabel( _( "Discard New Sheet" ) ) );
if( dlg.ShowModal() == wxID_NO )
return false;
m_showSheetFileNameCaseSensitivityDlg = !dlg.IsCheckBoxChecked();
}
return true;
}