public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
* setup.exe (cinstall) bugfixes + minor new feature
@ 2002-03-03 10:01 Max Bowsher
  0 siblings, 0 replies; 10+ messages in thread
From: Max Bowsher @ 2002-03-03 10:01 UTC (permalink / raw)
  To: cygwin


[-- Attachment #1.1: Type: text/plain, Size: 950 bytes --]

I've been working with the setup code, and have discovered some bugs in the
current (just updated) CVS version.

I'm posting a patch here for comments, whilst I join cygwin-patches, and
study the Contributing instructions.

The patch does the following:

BugFix: io_stream::mkpath_p(isadir, path) misuse
mkpath_p is supposed to take a path with either a file:// or a cygfile://
prefix, but it is fed a path with no prefix in some places. The patch adds a
file:// prefix where needed.
This was causing some directories not to be created, including the Cygwin
directory in the start menu.

BugFix: add backslash call to make_link, in desktop.cc
This was causing the start menu shortcut to be called
'Programs/Cygwin/Cygwin Bash Shell' instead of being in the correct
directory structure

Feature Addition: Use files /etc/setup/inhibit-{startmenu,desktop}-icon to
remeber user de-selection of the create icon checkboxes on the last page of
setup.

Max.

[-- Attachment #1.2: currwork.patch --]
[-- Type: application/octet-stream, Size: 2797 bytes --]

diff -mru -X patch-excludes src/winsup/cinstall/desktop.cc src-work/winsup/cinstall/desktop.cc
--- src/winsup/cinstall/desktop.cc	Sun Mar  3 17:29:24 2002
+++ src-work/winsup/cinstall/desktop.cc	Sun Mar  3 17:23:41 2002
@@ -103,7 +103,7 @@
 static void
 make_link (String const &linkpath, String const &title, String const &target)
 {
-  String fname = linkpath + "/" + title + ".lnk";
+  String fname = backslash(linkpath + "/" + title + ".lnk");
 
   if (_access (fname.cstr_oneuse(), 0) == 0)
     return;			/* already exists */
@@ -111,7 +111,7 @@
   msg ("make_link %s, %s, %s\n",
        fname.cstr_oneuse(), title.cstr_oneuse(), target.cstr_oneuse());
 
-  io_stream::mkpath_p (PATH_TO_FILE, fname);
+  io_stream::mkpath_p (PATH_TO_FILE, String("file://") + fname);
 
   String exepath;
 
@@ -278,7 +278,7 @@
 make_passwd_group ()
 {
   String fname = cygpath ("/etc/postinstall/passwd-grp.bat");
-  io_stream::mkpath_p (PATH_TO_FILE, fname);
+  io_stream::mkpath_p (PATH_TO_FILE, String("file://") + fname);
 
   if (uexists ("/etc/passwd") && uexists ("/etc/group"))
     return;
@@ -339,14 +339,27 @@
   make_etc_profile ();
   make_passwd_group ();
 
+  String inhibit_menu_file = backslash(cygpath("/etc/setup/inhibit-startmenu-icon"));
+  String inhibit_desktop_file = backslash(cygpath("/etc/setup/inhibit-desktop-icon"));
+
   if (root_menu)
     {
       start_menu ("Cygwin Bash Shell", batname);
+      _unlink(inhibit_menu_file.cstr_oneuse());
+    }
+  else
+    {
+      fclose(fopen(inhibit_menu_file.cstr_oneuse(),"wb"));
     }
 
   if (root_desktop)
     {
       desktop_icon ("Cygwin", batname);
+      _unlink(inhibit_desktop_file.cstr_oneuse());
+    }
+  else
+    {
+      fclose(fopen(inhibit_desktop_file.cstr_oneuse(),"wb"));
     }
 }
 
@@ -397,6 +410,9 @@
   if (_access (fname.cstr_oneuse(), 0) == 0)
     return 0;			/* already exists */
 
+  if (uexists("/etc/setup/inhibit-desktop-icon"))
+    return 0;			/* inhibited by previous user choice */
+
   return IDC_ROOT_DESKTOP;
 }
 
@@ -430,6 +446,9 @@
 
   if (_access (fname.cstr_oneuse(), 0) == 0)
     return 0;			/* already exists */
+
+  if (uexists("/etc/setup/inhibit-startmenu-icon"))
+    return 0;			/* inhibited by previous user choice */
 
   return IDC_ROOT_MENU;
 }
diff -mru -X patch-excludes src/winsup/cinstall/localdir.cc src-work/winsup/cinstall/localdir.cc
--- src/winsup/cinstall/localdir.cc	Mon Feb 18 13:53:06 2002
+++ src-work/winsup/cinstall/localdir.cc	Sun Mar  3 17:20:34 2002
@@ -45,7 +45,7 @@
 void
 save_local_dir ()
 {
-  io_stream::mkpath_p (PATH_TO_DIR, local_dir);
+  io_stream::mkpath_p (PATH_TO_DIR, String("file://") + local_dir);
 
   io_stream *f;
   if (get_root_dir ().size())

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2688 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* RE: setup.exe (cinstall) bugfixes + minor new feature
@ 2002-03-03 13:15 Robert Collins
  0 siblings, 0 replies; 10+ messages in thread
From: Robert Collins @ 2002-03-03 13:15 UTC (permalink / raw)
  To: Max Bowsher, cygwin

Thanks Max.

> -----Original Message-----
> From: Max Bowsher [mailto:maxb@ukf.net] 
> Sent: Monday, March 04, 2002 4:54 AM
> To: cygwin@cygwin.com
> Subject: setup.exe (cinstall) bugfixes + minor new feature
> 
> 
> I've been working with the setup code, and have discovered 
> some bugs in the current (just updated) CVS version.
> 
> I'm posting a patch here for comments, whilst I join 
> cygwin-patches, and study the Contributing instructions.
> 
> The patch does the following:
> 
> BugFix: io_stream::mkpath_p(isadir, path) misuse
> mkpath_p is supposed to take a path with either a file:// or 
> a cygfile:// prefix, but it is fed a path with no prefix in 
> some places. The patch adds a file:// prefix where needed. 
> This was causing some directories not to be created, 
> including the Cygwin directory in the start menu.

I know about these - they are fixed in setup200202 - when that goes live
I'll be backporting some key bugs. 
 
> BugFix: add backslash call to make_link, in desktop.cc
> This was causing the start menu shortcut to be called 
> 'Programs/Cygwin/Cygwin Bash Shell' instead of being in the 
> correct directory structure

Does this affect setup200202? I haven't checked yet. I'm trying to
encpasulate the path specific knowledge - so whilst this solution works,
I'd rather refactor make_link to leverage io_stream, and make this a
method or variant of the file:// io_stream. Also '/' separated paths are
valid to pass to the WIN32 API, so I'm curious why this is suddenly
become a problem. (Quite a lot of setup expects '/' separated paths, and
I see no reason to change that at this point.
 
> Feature Addition: Use files 
> /etc/setup/inhibit-{startmenu,desktop}-icon to remeber user 
> de-selection of the create icon checkboxes on the last page of setup.

This approach is too simple - it will only remember turning them off.
The settings should go in /etc/setup/setup.conf as something like
"desktop_icon = yes|no". 

Thanks for the patch though, I look forward to an update.

Cheers,
Rob

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: setup.exe (cinstall) bugfixes + minor new feature
@ 2002-03-04  2:08 Max Bowsher
  0 siblings, 0 replies; 10+ messages in thread
From: Max Bowsher @ 2002-03-04  2:08 UTC (permalink / raw)
  To: cygwin

[-- Attachment #1: Type: text/plain, Size: 2762 bytes --]

I did notice that setup-20020225.exe does not have the shortcut creation
bug - do you have some fixes which haven't made it to CVS yet?

Re Feature Addition - is the objection that you don't want to set a
precedent for lots of placeholder files in /etc/setup ? This I can
understand, but I don't see what 'it will only remember turning them off'
means - what else should it remember?
 
 
Max.
 
----- Original Message -----
From: "Robert Collins" <robert.collins@itdomain.com.au>
To: "Max Bowsher" <maxb@ukf.net>; <cygwin@cygwin.com>
Sent: Sunday, March 03, 2002 9:15 PM
Subject: RE: setup.exe (cinstall) bugfixes + minor new feature
> 
> 
> Thanks Max.
> 
> > -----Original Message-----
> > From: Max Bowsher [mailto:maxb@ukf.net]
> > Sent: Monday, March 04, 2002 4:54 AM
> > To: cygwin@cygwin.com
> > Subject: setup.exe (cinstall) bugfixes + minor new feature
> >
> >
> > I've been working with the setup code, and have discovered
> > some bugs in the current (just updated) CVS version.
> >
> > I'm posting a patch here for comments, whilst I join
> > cygwin-patches, and study the Contributing instructions.
> >
> > The patch does the following:
> >
> > BugFix: io_stream::mkpath_p(isadir, path) misuse
> > mkpath_p is supposed to take a path with either a file:// or
> > a cygfile:// prefix, but it is fed a path with no prefix in
> > some places. The patch adds a file:// prefix where needed.
> > This was causing some directories not to be created,
> > including the Cygwin directory in the start menu.
> 
> I know about these - they are fixed in setup200202 - when that goes live
> I'll be backporting some key bugs.
> 
> > BugFix: add backslash call to make_link, in desktop.cc
> > This was causing the start menu shortcut to be called
> > 'Programs/Cygwin/Cygwin Bash Shell' instead of being in the
> > correct directory structure
> 
> Does this affect setup200202? I haven't checked yet. I'm trying to
> encpasulate the path specific knowledge - so whilst this solution works,
> I'd rather refactor make_link to leverage io_stream, and make this a
> method or variant of the file:// io_stream. Also '/' separated paths are
> valid to pass to the WIN32 API, so I'm curious why this is suddenly
> become a problem. (Quite a lot of setup expects '/' separated paths, and
> I see no reason to change that at this point.
> 
> > Feature Addition: Use files
> > /etc/setup/inhibit-{startmenu,desktop}-icon to remeber user
> > de-selection of the create icon checkboxes on the last page of setup.
> 
> This approach is too simple - it will only remember turning them off.
> The settings should go in /etc/setup/setup.conf as something like
> "desktop_icon = yes|no".
> 
> Thanks for the patch though, I look forward to an update.
> 
> Cheers,
> Rob
> 
> 
> 


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2688 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* RE: setup.exe (cinstall) bugfixes + minor new feature
@ 2002-03-04  2:32 Robert Collins
  2002-03-04  5:01 ` Max Bowsher
  0 siblings, 1 reply; 10+ messages in thread
From: Robert Collins @ 2002-03-04  2:32 UTC (permalink / raw)
  To: Max Bowsher, cygwin



> -----Original Message-----
> From: Max Bowsher [mailto:maxb@ukf.net] 
> Sent: Monday, March 04, 2002 9:08 PM
> To: cygwin@cygwin.com
> Subject: Re: setup.exe (cinstall) bugfixes + minor new feature
> 
> 
> I did notice that setup-20020225.exe does not have the 
> shortcut creation bug - do you have some fixes which haven't 
> made it to CVS yet?
> 
> Re Feature Addition - is the objection that you don't want to 
> set a precedent for lots of placeholder files in /etc/setup ? 
> This I can understand, but I don't see what 'it will only 
> remember turning them off' means - what else should it remember?

I was thinking that 
If the setting is absent it prompts,
if the setting is on it always creates, overwriting the current one
if the setting is off it never creates.

Rob

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

^ permalink raw reply	[flat|nested] 10+ messages in thread
* RE: setup.exe (cinstall) bugfixes + minor new feature
@ 2002-03-04  2:34 Robert Collins
  2002-03-04  3:32 ` Max Bowsher
  0 siblings, 1 reply; 10+ messages in thread
From: Robert Collins @ 2002-03-04  2:34 UTC (permalink / raw)
  To: Max Bowsher, cygwin



> -----Original Message-----
> From: Max Bowsher [mailto:maxb@ukf.net] 
> Sent: Monday, March 04, 2002 9:08 PM
> To: cygwin@cygwin.com
> Subject: Re: setup.exe (cinstall) bugfixes + minor new feature
> 
> 
> I did notice that setup-20020225.exe does not have the 
> shortcut creation bug - do you have some fixes which haven't 
> made it to CVS yet?

No, its all checked in.

Rob

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

^ permalink raw reply	[flat|nested] 10+ messages in thread
* RE: setup.exe (cinstall) bugfixes + minor new feature
@ 2002-03-04  5:29 Robert Collins
  2002-03-04  8:18 ` Max Bowsher
  0 siblings, 1 reply; 10+ messages in thread
From: Robert Collins @ 2002-03-04  5:29 UTC (permalink / raw)
  To: Max Bowsher, cygwin



> -----Original Message-----
> From: Max Bowsher [mailto:maxb@ukf.net] 
> 
> 
> > I was thinking that
> > If the setting is absent it prompts,
> > if the setting is on it always creates, overwriting the 
> current one if 
> > the setting is off it never creates.
> 
> > Rob
> 
> Hmm - I'm not sure I understand this.
> 
> Current behaviour is that setup checks the boxes by default, 
> if it does not find the shortcuts. The user can manually 
> check the boxes to run the creation code even if they already 
> exist. I find it mildly annoying to have to uncheck both 
> boxes each time I run setup. (My desktop shortcut is called 
> something different, and I don't want a start menu one) 
> Therefore, I would like setup to remember the fact that the 
> user has deliberately unchecked the boxes. I don't understand 
> why anyone would want the shortcuts deleted and recreated 
> every time the run setup?

I can think of reasons :}. The first one being that as a sysadmin I
might want to force every user to get shortcuts - without being
prompted. Now a shared cygwin install does not imply shared profiles -
desktop and start menu locations - so the ability to turn off the prompt
and always create is thus useful.
 
> I will proceed with fixing that bug, and investigate the use 
> of a setup.conf file. Are there any objections to using 
> windows {Get,Set}PrivateProfileInt API calls? Or should I 
> investigate the setup.ini parsing code, and try to use that?

Neither :}. I've a model in mind for persistence for all the currently
diverse setup options. Seriously though, for now, code it with
Get|Set... And I'll get my model documented and into code at some point.

Rob

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

^ permalink raw reply	[flat|nested] 10+ messages in thread
* RE: setup.exe (cinstall) bugfixes + minor new feature
@ 2002-03-04 13:11 Robert Collins
  0 siblings, 0 replies; 10+ messages in thread
From: Robert Collins @ 2002-03-04 13:11 UTC (permalink / raw)
  To: Max Bowsher, cygwin



> -----Original Message-----
> From: Max Bowsher [mailto:maxb@ukf.net] 
> 
> But the user will need to run setup.exe and follow it through 
> all the way anyway. If they've managed to get through package 
> selection, does forcing the choice of shortcut creation help? 
> Or do you have big things planned for setup (full unattended 
> setup from an answer file?)?.

Yes.
 
> 
> If this is going to be interim only, do you mind if I stick 
> with my inhibit-?-icon files?

And yes.

The point being that the persistence logic may change, but the trinary
decision logic shouldn't.

Rob

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2002-03-04 21:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-03-03 10:01 setup.exe (cinstall) bugfixes + minor new feature Max Bowsher
2002-03-03 13:15 Robert Collins
2002-03-04  2:08 Max Bowsher
2002-03-04  2:32 Robert Collins
2002-03-04  5:01 ` Max Bowsher
2002-03-04  2:34 Robert Collins
2002-03-04  3:32 ` Max Bowsher
2002-03-04  5:29 Robert Collins
2002-03-04  8:18 ` Max Bowsher
2002-03-04 13:11 Robert Collins

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).