Closed Bug 781346 Opened 7 years ago Closed 7 years ago

Expose local profile directory

Categories

(Toolkit :: OS.File, defect, blocker)

defect
Not set
blocker

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: Yoric, Assigned: AMoz)

References

Details

(Whiteboard: [mentor=Yoric][lang=c++])

Attachments

(1 file, 4 obsolete files)

Bug 775588 exposes the profile directory (aka ProfD), which may be remote under Windows. We should expose the local profile directory (aka ProfLD).
By the way, what do ProfD and ProfLD yield on Android?
Whiteboard: [mentor=Yoric][lang=c++]
can I be assigned this project???
With pleasure. Note, however, that this project will have to wait a few days until we have found the problem with bug 775588.
Assignee: nobody → mailto.rajesh05
Depends on: 775588
Thanks a lot. Can you please tell approx time???
I think that I have found a fix for bug 775588. If testing confirms that my fix is correct, I hope that the patch will have landed by Wednesday.
(In reply to rajesh kumar from comment #4)
> Thanks a lot. Can you please tell approx time???

It has landed.
Rajesh, are you still planning to work on this?
Assignee: mailto.rajesh05 → nobody
I am interested to contribute to this bug. As explained by Mr. Yoric in irc, i got to know the concepts but i want to know what modifications need to be done in the corresponding files.
@Amod, the relevant code is in OSFileConstants.cpp.

As you may see in that code, we fetch the special directory NS_APP_USER_PROFILE_50_DIR to define a JavaScript constant |profileDir|. This is the global profile directory.

The objective of this bug is to also fetch the special directory NS_APP_USER_PROFILE_LOCAL_50_DIR to define a JavaScript constant |localProfileDir|. This will be the local profile directory.

You may find the documentation of most of the functions used in this file at https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/

You will then need to write a test. For this purpose, extend function |test_path| from main_test_osfile_async.js with a check almost identical to that of |profileDir|, but using |localProfileDir| and "ProfLD" instead of |profileDir| and "ProfD".
i have just replicated the stmnts with few minor changes. Also i have to write the test function..but before that let me know whether the initial part sounds good or not.
i dont understand why the diff view looks so strange when i have changed only few lines.
(In reply to Amod from comment #11)
> i dont understand why the diff view looks so strange when i have changed
> only few lines.

The reference version seems to be a very old version. Have you pulled recently?
Comment on attachment 673373 [details] [diff] [review]
this is a very crude patch...may contain many errors

Review of attachment 673373 [details] [diff] [review]:
-----------------------------------------------------------------

The result looks good. You will need to write a test, of course.
Could you first resolve the hg-related issue, though? Maybe you forgot to call |hg update| after |hg pull|?
Attachment #673373 - Flags: feedback+
Actually i didn’t use any hg command to get the code. I browsed to the file stored on the server and downloaded that (since it was only a matter of single file).
Amod, as I mentioned over IRC, you really should start using hg. Otherwise, you will end up with issues like this one.

https://developer.mozilla.org/en-US/docs/Developer_Guide/Source_Code/Mercurial
i tried a lot but on the command..
hg pull
the outcome is 
abort: no default repository found.

i tried this being in mozilla-central folder and once in mozilla-central/src folder.
Initially there was no src folder so i created it also i created the files /.hg/hgrc and wrote :

[paths]
default = http://hg.mozilla.org/mozilla-central/

now what else should i do ?
I even tried contacting in irc but didn’t get any solution.
has solved the issue thanks to jdm.
But now the connection times out a lot while executing the hg cmd.
that will be solve soon. 
thanks !
Solving the problem !
Currently i have exams till 3rd, so will continue the work after that. Thanks !
i used hg pull over other system and copied the entire folder on my system...Still that is giving the same patch..
That sounds surprising. Did you pull from mozilla-central?
yes. 
But still one thing is probing my mind. if the patch is getting compared with the old version on the server, then how come the client is at fault ?
The server is used only to download the latest version to the client. The patch is always compared with the version on the client. Consequently, the issue must be on the client – or in the command used to generate the patch.

* Normally, after your pull, the version of OSFileConstants.cpp that you have on your computer is identical to the following: dxr.mozilla.org/mozilla-central/dom/system/OSFileConstants.cpp.html – check the definition of |typedef struct { ... } Paths| to ensure that this is correct. If the version that you have has a different version of |Paths|, then you have the wrong version on your computer, and you should fix this.

* Create a new patch with |hg qnew|, e.g. |hg qnew localProfileDirectory|.

* Make your changes to OSFileConstants.cpp .

* Record your changes to the patch with |hg qref|.

* Attach the patch to this bug.
the typedef struct{ //stmnts } is very much similar to that of the dxr path mentioned above but when i use diff /path of the file > patch_name, it compares with the older version.

i have PASTEBINed the typedef struct {} on the following link. 
http://pastebin.mozilla.org/1911255 




(In reply to David Rajchenbach Teller [:Yoric] from comment #22)
> The server is used only to download the latest version to the client. The
> patch is always compared with the version on the client. Consequently, the
> issue must be on the client – or in the command used to generate the patch.
> 
> * Normally, after your pull, the version of OSFileConstants.cpp that you
> have on your computer is identical to the following:
> dxr.mozilla.org/mozilla-central/dom/system/OSFileConstants.cpp.html – check
> the definition of |typedef struct { ... } Paths| to ensure that this is
> correct. If the version that you have has a different version of |Paths|,
> then you have the wrong version on your computer, and you should fix this.
> 
> * Create a new patch with |hg qnew|, e.g. |hg qnew localProfileDirectory|.
> 
> * Make your changes to OSFileConstants.cpp .
> 
> * Record your changes to the patch with |hg qref|.
> 
> * Attach the patch to this bug.
Good, that's the right definition of |Path|.

Now, I don't know how you use |diff|, but you should use |hg qnew|/|hg qref|, as I mentioned above. This should solve the issue.

I forgot to mention that you will find the file containing the patch in directory .hg/patches
Ok, this bug is now officially blocking me from progressing with bug 753768. Amod, our discussion on IRC seem to indicate that you are basically done with this bug. Is there anything still missing?
Severity: enhancement → blocker
i have to upload the final patch which will extend the | test | function which i will upload within a day.
If you require it very urgently then you may proceed.
Thanks !
Attached patch The code (obsolete) — Splinter Review
Hello. 
You earlier mentioned that i need to | extend | function from the js file. I thought there is something | inheritance | involved since i took the word "extend" in that manner (as used in c++ to extend a class).
So far these days i had been searching google and MDN in and out on how to do this until i saw your comment and finally asked on IRC.
jdm told me that what i was interpreting totally different than what you wanted to say.

So was the delay...i would have uploaded the patch far back had i not got confused. You also know in the previous patch that i made the appropriate changes.
Attachment #673373 - Attachment is obsolete: true
Sorry about the confusion. Thanks for the patch, I will review it.
Comment on attachment 684499 [details] [diff] [review]
The code

Review of attachment 684499 [details] [diff] [review]:
-----------------------------------------------------------------

C++ looks good, but I am still waiting for the tests.
Attachment #684499 - Flags: review+
which automated test should i opt for ?
The test I asked you to patch is main_test_osfile_async.js, function |test_path|. This test is a chrome mochitest. To launch it:

./mach mochitest-chrome toolkit/components/osfile/tests/mochi/main_test_osfile_async.js
i have exectured the cmd which you mentioned at irc.
It gave following results. I have pastebined upto what the terminal in ubuntu could hold.
Have a look at it.

http://pastebin.mozilla.org/1967164
i have executed the cmd which you mentioned at irc.
It gave following results. I have pastebined upto what the terminal in ubuntu could hold.
Have a look at it.

http://pastebin.mozilla.org/1967164
Amod, I asked you to patch the file, so that we can add it to our test suite. This will allow us to execute the test automatically, on all platforms, with each change to Firefox, to ensure not only that the feature works now but also that it keeps working despite successive changes to Firefox.

So, could you please provide the patch I am asking for?
I have already provided you the patch earlier...then you had told me to do the test which i did just now.
I had modified only two files: one is OSConstants.cpp and main_test_osfile_async.js out of which i have provided the patch of OSConstants.cpp
Attached patch test patch. (obsolete) — Splinter Review
i added a stmnt to the test_path as said by you.
Comment on attachment 686487 [details] [diff] [review]
test patch.

Review of attachment 686487 [details] [diff] [review]:
-----------------------------------------------------------------

That patch just adds a blank line. Are you sure it's the right patch?
Attached patch The test (obsolete) — Splinter Review
apologizes..this is with the required changges !
Attachment #686487 - Attachment is obsolete: true
Comment on attachment 686495 [details] [diff] [review]
The test

Review of attachment 686495 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, that one looks good.
Attachment #686495 - Flags: review+
what work is remaining now regarding this bug ?
Attachment #684499 - Attachment description: contains all the changes you have mentioned ! → The code
Attachment #686495 - Attachment description: test patch. → The test
At this stage, we still need to:
- merge with the latest versions of m-c;
- launch tests on all platforms;
- format the patches correctly.

As I need this patch, I will take over from this point.
Same patches, consolidated as one patch, and following the conventions of http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed
Attachment #684499 - Attachment is obsolete: true
Attachment #686495 - Attachment is obsolete: true
Attachment #686544 - Flags: review+
Assignee: nobody → amod.narvekar
Tests pass. Marking checkin-needed.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e7ac63e9cbf1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.